frankiesardo / icepick

Android Instance State made easy
Eclipse Public License 1.0
3.75k stars 208 forks source link

Support null values #31

Closed MFlisar closed 9 years ago

MFlisar commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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-- commented 9 years ago

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

clemp6r commented 9 years ago

Any update?

frankiesardo commented 9 years ago

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

frankiesardo commented 9 years ago

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