mcharmas / android-parcelable-intellij-plugin

IntelliJ Plugin for Android Parcelable boilerplate code generation.
Apache License 2.0
2.14k stars 214 forks source link

Add support for type Map #19

Open mpost opened 10 years ago

mpost commented 10 years ago

While it is already in the TODO list i wanted to raise my voice and mentioned that Map support would be really great. :)

mcharmas commented 10 years ago

Sorry, no time to add it in neares future. Implementing this should not be that hard. If you have time and will please issue pull request. If you have any questions please ask.

mpost commented 10 years ago

Thanks for the feedback. What would you suggest to add that feature? In my implementation I used two lists. One for keys and one for values. Am 09.07.2014 18:10 schrieb "Michał Charmas" notifications@github.com:

Sorry, no time to add it in neares future. Implementing this should not be that hard. If you have time and will please issue pull request. If you have any questions please ask.

— Reply to this email directly or view it on GitHub https://github.com/mcharmas/android-parcelable-intellij-plugin/issues/19#issuecomment-48496418 .

dallasgutauckis commented 10 years ago

@mpost That's a pretty common way to go about doing it, and how I would do it if I had time to work on it. Feel free to submit a PR, I can nitpick anything that needs it and if @mcharmas has the time he can review as well and merge + deploy.

mpost commented 10 years ago

Okay. I have never worked on any intellij code so I can't promise anything. Hope to find the time.

dallasgutauckis commented 10 years ago

@mpost neither had I… nor could I promise anything. Just give it a whirl and if you come up with something, PR it ;-)

Thanks!

mcharmas commented 10 years ago

Neither had I before this plugin... The problem is that its not that obvious how to put map into parcel. There are several ways how to do it. The best solution would be to reuse existing serializes to do it and support all possible key/value types. There is a discussion on SO how to do it.

http://stackoverflow.com/questions/8254654/how-write-java-util-map-into-parcel-in-a-smart-way

What do you think about it?

mpost commented 10 years ago

In my implementation i knew that both key and value would be of type string so i serialized into lists of String.

In general i would think that both key and value have to be of type Parcelable (or Serializable although that is discouraged for performance reasons).

mcharmas commented 10 years ago

The solution should be generic.

I like this solution becouse already existing ChainSerializerFactory (the same as the one declared in CodeGenerator.java) can be used (I think so).

public void writeToParcel(Parcel out, int flags){
  out.writeInt(map.size());
  for(String key : map.keySet()){
    out.writeString(key); // to generate this use SerializerFactory that supports all types except map
    out.writeString(map.get(key)); // the same for this one
  }
}

private MyParcelable(Parcel in){
  //initialize your map before
  int size = in.readInt();
  for(int i = 0; i < size; i++){
    String key = in.readString(); // use factory to deserialize
    String value = in.readString(); // use factory to deserialize
    map.put(key,value);
  }
}

In such solution it would be possible to reuse serialization implementation of all types already supported by tools - both as key and value.

I have a feelint that it may be possible without major refactoring.

mpost commented 10 years ago

I have the impression that somebody has a good idea on how to implement the map support and just voluntered for the job. 😀

dallasgutauckis commented 10 years ago

Hmm, didn't remember this was here before I implemented what I did. I didn't refactor the ChainSerializerFactory though because, well, there's a lot of custom stuff that needs to be done to get this quite right and didn't feel the need to get it perfect on this run. I'm wondering if we can even do it without a more significant refactor of TypeSerializer as that interface currently is using PsiField which a generic will not be.

I have a PR currently up with some Map support — would appreciate it if that were more heavily tested before the plugin is deployed.

Aditya94A commented 6 years ago

...

~3.5 years later, any updates?!

I was banging my head on the wall for weeks after finally running into this SO question with the exact answer I needed. (TL;DR: This plugin didn't support Map so we have to take care of it ourselves).

For future Googlers, replace the writeSerializable and readSerializable calls auto-generated by this plugin with calls to the following methods

// For writing to a Parcel
public <K extends Parcelable,V extends Parcelable> void writeParcelableMap(
        Parcel parcel, int flags, Map<K, V > map)
{
    parcel.writeInt(map.size());
    for(Map.Entry<K, V> e : map.entrySet()){
        parcel.writeParcelable(e.getKey(), flags);
        parcel.writeParcelable(e.getValue(), flags);
    }
}

// For reading from a Parcel
public <K extends Parcelable,V extends Parcelable> Map<K,V> readParcelableMap(
        Parcel parcel, Class<K> kClass, Class<V> vClass)
{
    int size = parcel.readInt();
    Map<K, V> map = new HashMap<K, V>(size);
    for(int i = 0; i < size; i++){
        map.put(kClass.cast(parcel.readParcelable(kClass.getClassLoader())),
                vClass.cast(parcel.readParcelable(vClass.getClassLoader())));
    }
    return map;
}

I can finally stop using Serializable. Good riddance.