Skip to content

Conversation

davidgrupp
Copy link
Contributor

Added support to JsonDeserializer for deserializing a Dictionary to a Dictionary<String,List>

@haacked
Copy link
Contributor

haacked commented Oct 16, 2013

Mind adding a unit test for this? Also, we use the SimpleJson deserializer. So it might be good to submit a PR to this : https://github.com/facebook-csharp-sdk/simple-json so we don't have to special case it here. :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're only converting but not adding in the foreach loop. Therefore only the last item would be added to the dictionary.

@davidgrupp
Copy link
Contributor Author

@haacked Yea i'll add unit test(s) for this.

@thoemmi This is a good point, however, the code does work as expected. I think instead of the foreach it just needs to take the first element of Child.Value and covert that value.

@haacked
Copy link
Contributor

haacked commented Oct 17, 2013

This is a good point, however, the code does work as expected.

Make sure your tests include a case where the value is a JsonArray with more than one element.

…t calling BuildList method which fixed confusion around list of lists.

Also updated the sample data to more clearly show lists vs list of lists
@davidgrupp
Copy link
Contributor Author

Is there anything else that needs to be addressed for this PR? Are the unit test cases clear?

haacked added a commit that referenced this pull request Oct 21, 2013
Added support to JsonDeserializer for deserializing a Dictionary to a Dictionary<String,List<T>>
@haacked haacked merged commit d834b72 into restsharp:master Oct 21, 2013
@haacked
Copy link
Contributor

haacked commented Oct 21, 2013

Thanks for your contribution!

thumbs-up-colbert

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

Successfully merging this pull request may close these issues.

3 participants