facebook-csharp-sdk / simple-json

JSON library for .NET 2.0+/SL4+/WP7/WindowsStore with optional support for dynamic and DataContract
MIT License
380 stars 143 forks source link

Deserialize lists that directly implement list interfaces #38

Open haacked opened 10 years ago

haacked commented 10 years ago

This is related to #36.

I wondered what would happen if you manually implemented a list like so:

public class NumberList : IList<int> {...}

Turns out that fails to deserialize even if the list is a property of another object. The reason is in this code:

Specifically this clause:

ReflectionUtils.IsAssignableFrom(typeof(IList), type)

ICollection<T> and IList<T> do not implement IList. The concrete types do, but the interfaces do not.

So it seems to me we should check against IList and ICollection<T> where T is the element type and then cast the result to the correct one.

I can submit a fix if this is something you'd take. I didn't actually run into this in my own code, so it's not urgent to me. But it's something I realized as I was thinking about the various cases.

prabirshrestha commented 10 years ago

I would personally be fine with merging it in (especially if it is just few changes to the codebase).

But I don't see any reason why someone would want to implement IList<T>. (SimpleJson is really meant to do simple things. POCOs should be enough. With .NET extension methods one can easily sort of extend IList<T>.)

If you do run into a situation where you might need it, let us know and we can look into merging it.

haacked commented 10 years ago

But I don't see any reason why someone would want to implement IList<T>.

Maybe they're not the ones implementing it. Suppose you have an object with a property of type IPAddressCollection.

Now, I don't have this problem and I think 99% of folks won't, so I'm happy to ignore this until someone asks for it. :)