Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for deserializing properties/fields of collection interfaces #3

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

rbeurskens
Copy link

No description provided.

…face types for collections that were already supported (Lists and Dictionaries). ISet<T> and non-generic IDictionary are still unsupported.
@mgholam
Copy link
Owner

mgholam commented Aug 18, 2017

Thanks!

Personally I would cache the MakeGenericType since it is a performance hog, have you tested the performance?

@rbeurskens
Copy link
Author

No, not yet. I will take a look at the performance next week.

… a generic collection type that implements an interface (this can be used to for other generic types, too)
@mgholam
Copy link
Owner

mgholam commented Aug 21, 2017

Did you get any timing difference?

@mgholam
Copy link
Owner

mgholam commented Aug 21, 2017

Also instead of Tuple try using a struct

@rbeurskens
Copy link
Author

rbeurskens commented Aug 21, 2017

In my own project I'm using C#7 System.ValueTuple (which is a struct), but I don't know what the minimum c# / .net version is that should be supported. I found that with some additions, it could work for .NET 4.0, but I'm not sure about IDE support. I'll just add a custom struct instead to be sure I'm not using language features that are 'too new'.

@rbeurskens
Copy link
Author

rbeurskens commented Aug 21, 2017

(I have not tested if caching improves performance, but I'm quite confident a dictionary lookup will be faster than repeated calls to MakeGenericType. However, it will only make a difference when the type is used more than once, of course. I could also make the dictionary creation lazy, as it won't always be used. )

@rbeurskens
Copy link
Author

rbeurskens commented Oct 16, 2017

Note: there's still a bug when deserializing ISet<T>: An instance of List<T> is used, bypassing the type system as IL is used in the setter, causing EntryPointNotFoundException to be thrown when accessing members of ISet<T>. (also none of the ISet<T> implementations are supported such as HashSet<T> and OrderedSet<T> )
Edit: this is now resolved with commit e015487

@rbeurskens
Copy link
Author

rbeurskens commented Aug 13, 2020

Sorry for the delay in resolving the conflicts. I switched jobs in the meantime and just found I had something unfinished. (I could not test if my changes still work after any changes on master in the meantime, but I did my best adjusting it just looking at the code )

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.

2 participants