8000 Allow to provide a default value for @DefaultStringSet by dodgex · Pull Request #1555 · androidannotations/androidannotations · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Allow to provide a default value for @DefaultStringSet #1555

Merged
WonderCsabo merged 2 commits into from
Sep 18, 2015
Merged

Allow to provide a default value for @DefaultStringSet #1555

WonderCsabo merged 2 commits into from
Sep 18, 2015

Conversation

@dodgex
Copy link
Member
@dodgex dodgex commented Sep 17, 2015

based on #1553

This PR allows to provide a default value for a @DefaultStringSet annotated method in a @SharedPref interface.

Usage:

    @DefaultStringSet({"a", "b", "c"})
    Set<String> setWithDefault();

That generates:

   public StringSetPrefField setWithDefault() {
        return stringSetField("setWithDefault", new HashSet<String>(Arrays.asList("a", "b", "c")));
    }

@WonderCsabo
Copy link
Member
WonderCsabo commented Sep 18, 2015

Seems to be cool, i added two comments. Could you please rebase this onto develop?

WonderCsabo added a commit that referenced this pull request Sep 18, 2015
@WonderCsabo
Merge pull request #1555 from dodgex/allow_to_provide_a_default_value…
877a31d 
…_for_DefaultStringSet

Allow to provide a default value for @DefaultStringSet
@WonderCsabo WonderCsabo merged commit 877a31d into androidannotations:develop Sep 18, 2015
@yDelouis yDelouis added this to the 4.0 milestone Sep 19, 2015
@dodgex dodgex deleted the allow_to_provide_a_default_value_for_DefaultStringSet branch September 20, 2015 15:47
@athkalia
Copy link
athkalia commented Nov 6, 2015

Hello,

I am having some trouble with this change, it seems that this default value is mandatory. Unfortunately, I am getting my default value through a

    <MultiSelectListPreference android:key="@string/prefernce_key"
                                   android:title="@string/preference_title"
                                   android:dialogTitle="@string/dialog_title"
                                   android:entries="@array/preference_entries"
                                  android:entryValues="@array/preference_entries_values"
                                   android:defaultValue="@array/preference_default_entry_values"/>

so this change doesn't quite suit me. Can't you keep it as optional ? (by the way the comment in the DefaultStringSet still claims that

 * <p>
 * Use on methods in {@link SharedPref} annotated class. The generated method
 * will return an empty {@link java.util.Set} of Strings by default.
 * </p>

Cheers

@WonderCsabo
Copy link
Member
WonderCsabo commented Nov 6, 2015

The JavaDoc has to be updated for sure. But we require the default value on any @DefaultXXX annotation. You can just specify an empty array, the behavior will be the same before this change. Alternativaly, you can just remove the @DefaultStringSet annotation, but that way you cannot specify keyRes.

@athkalia
Copy link
athkalia commented Nov 6, 2015

thanks for the response. I noticed that

value = {}

does not work. (it does work but generates code that doesn't compile)

value = ""

does work, but it's not quite the same.

In version 3.3.2
a

new HashSet<String>(0)

would get generated,
In version 4.0 with

value = ""

we get a

new HashSet<String>(Arrays.asList(""))

and with

value = {}

we get a

new HashSet<String>(Arrays.asList())

(doesn't compile)

@WonderCsabo
Copy link
Member
WonderCsabo commented Nov 6, 2015

Thanks for checking! "" is shortcut for {""} so that is okay. But we should
handle the empty array. @dodgex wanna fix this?
On Nov 7, 2015 00:47, "athkalia" notifications@github.com wrote:

thanks for the response. I noticed that

value = {}

does not work. (it does work but generates code that doesn't compile)

value = ''" does work, but it's not quite the same.

In version 3.3.2
a

new HashSet(0)

would get generated,
In version 4.0 with

value = ""

we get a

new HashSet(Arrays.asList(""))

and with


we get a
```new HashSet<String>(Arrays.asList())

(doesn't compile)

—
Reply to this email directly or view it on GitHub
https://github.com/excilys/androidannotations/pull/1555#issuecomment-154579420
.

@athkalia
Copy link
athkalia commented Nov 7, 2015

Last one, food for thought. Can't the @DefaultRes also add support for Set and link the default value to an R.array.* value ?

@dodgex
Copy link
Member Author
dodgex commented Nov 7, 2015

@WonderCsabo cant fix it before 15.11. and the i have to find time.

@dodgex dodgex mentioned this pull request Nov 14, 2015
Merged
@dodgex
Copy link
Member Author
dodgex commented Nov 14, 2015

i created PR #1624 with a fix and javadoc update

@WonderCsabo
Copy link
Member
WonderCsabo commented Nov 14, 2015

@athkalia the empty array is now fixed thanks to @dodgex.

Mapping Android resources to pref default values is the behavior of the @DefaultRes annotation. I think we can extend it by supporting a resource string array as default value of a string set. Please open a new issue for that.

@athkalia
Copy link
athkalia commented Nov 14, 2015

Lovely! Will do, thanks

@athkalia athkalia mentioned this pull request Nov 14, 2015
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews
54A3

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.0

Development

Successfully merging this pull request may close these issues.

4 participants

@dodgex @WonderCsabo @athkalia @yDelouis
0