Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add reasonable defaults for primitive preferences. #29

Merged
merged 1 commit into from
Oct 4, 2015

Conversation

f2prateek
Copy link
Owner

Adds reasonable defaults (false for booleans and 0 for
numbers) to the factory methods for primitve preference types.

Using null as the default for primitive preference types will trip
people if they try to automatically unwrap those values when a
preference doesn't exist.

@CheckResult @NonNull
public Preference<Integer> getInteger(@NonNull String key) {
return getInteger(key, null);
//noinspection UnnecessaryBoxing
return getInteger(key, Integer.valueOf(0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the inconsistent use of constants to cache these instances?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, should use Boolean.valueOf above. But probably want to cache Float ourselves, since it always returns a new instance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably cache them all in DEFAULT_<foo> static fields in this class to be explicit. Relying on the caching/interning of instance of Java seems a bit too implicit.

This commit adds reasonable defaults (`false` for booleans and `0` for
numbers) to the factory methods for primitve preference types.

Using `null` as the default for primitive preference types will trip
people if they try to automatically unwrap `null` values when a
preference doesn't exist.
f2prateek added a commit that referenced this pull request Oct 4, 2015
Add reasonable defaults for primitive preferences.
@f2prateek f2prateek merged commit 5d6b447 into master Oct 4, 2015
@f2prateek f2prateek deleted the null-safe-defaults branch October 4, 2015 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants