-
Notifications
You must be signed in to change notification settings - Fork 208
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
Added Parceler support #20
Conversation
I'm pretty confident the |
Supporting Parceler will allow users to use it's direct Parcelable serialization generated classes. It looks like you are already serializing beans using Gson, so in terms of the user's experience this would open up another Serialization option. |
A bean annotated with Paceler's annotation will always be serialized/deserialized by Icepick, so there's no need to add custom code to handle it. |
I don't understand, are you saying that Icepick will already serialize POJOs? I do not believe it will make the necessary calls into the Parceler api to serialize using Parceler's generated Parcelable classes. |
Yup, try it for yourself: your |
Right, but the technique of using Gson to marshal beans to Strings will not be as fast as using a Parcelable class directly (what Parceler generates for you). By the way, I am unable to build your library from the repository and it seems Travis is broken. Can you fix your build? |
I don't see any problem on the build on my side. What error are you seeing? |
+1 on this PR. If you're using Parceler already to pass objects around, it seems like you may as well use that same serialization with Icepick! And +1 to both of you for two awesome libraries that I would love to see working together :) |
Thanks @aroldan , maybe you can help convince @frankiesardo that this is a meaningful addition. I've also been considering adding something similar to Icepick into Parceler if that will help. |
I'd love to have a single solution in either case whether that's Parceler in Icepick or Icepick in Parceler. I'm using Jackson to handle serialization/deserialization in my apps so having three different persistence solutions seemed like overkill. I like the model where Icepick defines the syntax and the serialization/persistence part of it is pluggable to be Jackson/Gson/Parceler/whatever based on annotations or options. Ideally the end user doesn't have to care in most cases but when you do have to define custom serialization it'd be nice to only have to write it once. |
@aroldan I understand your needs, but with If you need to persist any value object |
The case to favor Parcelables over Serializables: http://www.developerphil.com/parcelable-vs-serializable/ Parceler just makes it easier to leverage Parcelables... plus Icepick already supports classes that extend Parcelable (I think). Therefore this would be a positive addition to Icepick. |
@frankiesardo we have a bunch of pojos and collections of pojos that need to be serialized.. they are already parcelized for passing around on intents and so forth.. allowing this PR would give us a performance improvement in save/restore where it's really important. A parcel has only values in it so I'm not sure I understand your last argument.. |
@dbachelder I have the belief that libraries should be tiny and focused and easy to compose. There's no way to compose Instead I suggest you use android-auto-value for your Parcelable objects. A part from being easier to use and to maintain, it also has faster runtime execution (no reflection) and it composes nicely with everything using Parcelable objects. In fact the model itself is |
Since the issue is closed I don't want to insist too much on that, but am also on @johncarl81 's side. I would really like to have a Parceler support in IcePick. Android-Auto-Value is a smart alternative too, but limited for my needs. Adding Parceler as an optional dependency in IcePick would not break it and would simplify my development, in addition to make it more efficient. |
@Nilhcem How is Regarding this PR, I'm sorry but I won't change my mind. I already stated all the good reasons why I don't want this merged in, and I ask you guys to respect it. You're always welcome to fork the project if you think this feature is much needed. |
I sometimes want to save the state of some complex Parcelable mutable objects (mostly Jackson annotated objects) without writing the Parcelable boilerplate. |
Oh Jackson integration is easy as one two three. You just annotate the static constructor with @AutoValue
public abstract class Person implements Parcelable {
@JsonCreator
public static Person create(@JsonProperty("name") String name, @JsonProperty("surname") String surname) {
return new AutoValue_Person(name, surname);
}
public abstract String name();
public abstract String surname();
}
Person mj = new ObjectMapper.readValue("{\"name\": \"Michael\", \"surname\": \"Jackson\"}", Person.class); |
I missed that, Thanks a lot! |
@frankiesardo, I respect your decision to focus on Immutability, but I want to point out another technique that may be of interest for integrating with external libraries like Parceler. On the FragmentArgs project, they introduced a converter parameter " |
Looks more like forcing people into doing so. @frankiesardo Can you say it honestly, you constantly reject any attempts to support Parceler only to protect your own library, AutoParcel? Because your reasoning is not convincing at all. People use different approaches to develop apps, there is no such thing as "the only right" way to write an app. It is often not possible to get along with the limitations that AutoParcel comes with. Supporting Parceler or even better, arbitrary serializers, would be a really great feature. A few more arguments for Parceler I can think of: when you save a state of a manually implemented Parcelable, how do you know it is immutable? The answer is you don't. The whole purpose of Parceler is to generate all the boilerplate code (same as Icepick btw). So why not think of it as Parcelable that just looks slightly different? Also Parceler doesn't imply any limitations on classes, unlike AutoParcel. Summarizing the above:
And the argument I think is incontestable: one uses Icepick to save state, and the state is mutable by definition. I hope you'll get my point. Icepick is a great library and it can be even more awesome if it were playing well with the others. P.S. Regarding this
AutoParcel is clearly violating this. Looks like hypocrisy to me. |
Also I want to mention that I've read you opinion on immutability in this and other issues, and you certainly have a point. But there are things like legacy code, company standards and many more that can prevent from doing things your way. One can simply disagree with you, and this is pretty much legal too. Again, you cannot control whether a Parcelable implemented by hand is immutable, so this is hardly an argument. People stubborn enough can even actually write the boilerplate and keep objects mutable anyway, and Icepick will serialize their objects without a hesitation. But more probably they'll keep using Parceler and will simply save parcels some other way. |
@ultimate-deej I think you make some good points (except the part where you accuse me of boycotting Parceler to promote AutoParcel, you could have avoided that), esp. regarding supporting arbitrary serialisers. It would be cool to have a mechanism like in google/auto to register extensions for custom annotation processing. What I think it's not cool is to add explicit calls to Although I wrote both icepick and auto-parcel, you won't see in either library code that reference the other. This is because they are small, focused and compose well. You could argue that Parceler, because it hides the Anyway, I'd be very happy to see Icepick and Parceler playing well together. The original solution of adding parceler specific code to icepick does not work for me: I don't want to depend on Parceler implementation. Adding custom extensions to icepick is cool but it requires a lot of work and if it's just for one library I would probably never do it. If there are any other ways to make this marriage work I'm open to suggestions. |
@frankiesardo, I'd recommend the technique that @sockeqwe used in FragmentArgs to integrate Parceler. Namely he added a converter interface that could be provided by the user: @FragmentWithArgs
public class MyFragment {
@Arg ( bundler = ParcelerArgsBundler.class)
Dog dog; // Dog is @Parcel annotated
} https://github.com/sockeqwe/fragmentargs#argsbundler This meets the requirement of keeping Parceler specific code out of icepick. Worth noting, because Parceler does convert to and from the class ExampleActivity extends Activity {
@State Parcelable parcelable; // This will be automatically saved and restored
@Override public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Icepick.restoreInstanceState(this, savedInstanceState);
}
@Override public void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
Icepick.saveInstanceState(this, outState);
}
public void useExample() {
ParcelerExample example = Parceler.unwrap(example);
//...
}
} This is similar to the suggested method of using Parceler with Android Annotations: http://stackoverflow.com/a/29691052/654187 until we integrated it directly. |
@frankiesardo Good to hear you are open to suggestions! As for Parceler, do you have any ideas how to modify it to make them work together? |
Yeah I like that @johncarl81, I would probably implement the same strategy for icepick. There's a good chance I'll get something done during the xmas break. @ultimate-deej for now you can use the unwrap strategy described above, |
With just a couple additions Icepick can support Parceler. This means that Icepick can read beans annotated with
@Parcel
if the Parceler library is included and used.Included are the necessary changes, examples and tests.
Forgive me if this doesn't work 100%, I was unable to get the Gradle build to execute.