Instagram / ig-json-parser

Fast JSON parser for java projects
https://instagram-engineering.com/fast-auto-generated-streaming-json-parsing-for-android-ab8e7be21033
MIT License
1.32k stars 125 forks source link

Support Map types #4

Closed Wavesonics closed 9 years ago

Wavesonics commented 10 years ago

Lists are supported, adding support for Map types would be very helpful.

ttung commented 10 years ago

What do you envision being parsed into maps?

Wavesonics commented 10 years ago

For parsing something like:

{
  "AMap" :
  [
    "Key1" : "value1",
    "Key2" : "value2"
  ]
}

Into:

class Foo
{
  public Map<String, String> aMap;
}
ttung commented 10 years ago

So a fixed type for the value?

Wavesonics commented 10 years ago

Yeah. Can't think of a good way to arbitrarily parse value objects. But having a map with a single value type would still solve most of the cases I think. Certainly my own cases.

jacobtabak commented 10 years ago

Check out GSON's implementation - https://github.com/eatnumber1/google-gson/blob/master/src/main/java/com/google/gson/MapTypeAdapter.java

Wavesonics commented 10 years ago

Taking my first baby steps with ig-json and it looks pretty great so far. I was going to take a crack at implementing parsing for map types, any suggestions on where to start, things to look out for?

ttung commented 10 years ago

@Wavesonics the logic would be pretty similar to what is done for list/sets right now.

Wavesonics commented 10 years ago

@ttung so I spent some time on this today, I have it deserializing String,String Maps (sort of), however the major hurdle is the fact that everything is written with the assumption that there would only ever be one parameterized type.

  TypeData.mParseType

So I'm working on refactoring JsonAnnotationProcessor.processFieldAnnotation() to be able to discern multiple parameterized types, and for TypeData to store a list rather than a single type. This has knock on effects I think with mPackageName and mParsableType also needing to be Lists.

This creates some complications in places that access those fields. However the larger concern is JsonParserClassData.mArraySerializeCalls(), currently it's a one-to-one mapping of type to string. That won't be feasible with multiple parameters as we'd need every permutation.

Having written the code originally, do you have any thoughts on a better way to support multiple type parameters?

ttung commented 10 years ago

@Wavesonics I'm curious why you think having multiple types is a good idea. How would the code determine which type to map to (scalars being exempted)?

ttung commented 9 years ago

this is supported as of https://github.com/Instagram/ig-json-parser/commit/374fca68675809ce19c1bfb7ce59c772a6c2c382. it has also been released to maven as the 0.0.5 release.