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

Support null values #31

Closed
MFlisar opened this issue Oct 20, 2014 · 6 comments
Closed

Support null values #31

MFlisar opened this issue Oct 20, 2014 · 6 comments

Comments

@MFlisar
Copy link

MFlisar commented Oct 20, 2014

I would suggest, that you generate code like following (example for Long, but should be used for all objects):

Saving:

if (target.icon != null)
    outState.putLong(BASE_KEY + "icon", target.icon);

Reading:

if (savedInstanceState.containsKey(BASE_KEY + "icon"))
    target.icon =  savedInstanceState.getLong(BASE_KEY + "icon");

Otherwise, you library which is really very useful, can't be used with null values...

@frankiesardo
Copy link
Owner

That's not an error that happens with every object.
It specifically happens for boxed primitives (e.g. Integer, Long..) which are not automatically initialised like their unboxed counterparts (int, long). Thanks for reporting the bug, I believe icepick should work around it rather than fail with NPE, but I strongly advise all users to either avoid using boxed primitives or give a default value when the variable is initialised.

@MFlisar
Copy link
Author

MFlisar commented Oct 21, 2014

Even if you advise not to use null values, the possibility to use them is always useful (and I personally have usecases where I don't want a default value for an object but rather a null value).

To avoid the unnecessary null check, it could be possible to use two annotations, one for objects that are never null and one for objects that can be null (like "@icicle" and "@IcicleNullable" or so)

Please tell me if you think this is something useful and if you plan to implement it. I've never worked with annotation processors, but I would look into it and extend the library with an extra annotation otherwise... Although, I probably need longer than you would :-)

@Alexander--
Copy link

I have created a merge request which adds sort of support for this. Feel free to comment on the implementation there.

@clemp6r
Copy link

clemp6r commented Jan 14, 2015

Any update?

@frankiesardo
Copy link
Owner

If @Alexander-- is not interested anymore in his PR, I will fix it on the fly myself. Expect a snapshot version soon(ish).

@frankiesardo
Copy link
Owner

Pushed a 2.3.7-SNAPSHOT artefact. It should just work now.

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

No branches or pull requests

4 participants