facebook-csharp-sdk/simple-json

Deserialize lists that directly implement list interfaces

Opened this issue · 2 comments

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.

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.

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. :)