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

Add serializer tests for value types #1211

Merged
merged 1 commit into from
Feb 27, 2020

Conversation

steveharter
Copy link
Member

No description provided.

@@ -24,6 +24,9 @@ namespace System.Text.Json.Serialization.Tests
[GenericTypeArguments(typeof(HashSet<string>))]
[GenericTypeArguments(typeof(ArrayList))]
[GenericTypeArguments(typeof(Hashtable))]
[GenericTypeArguments(typeof(SimpleStructWithProperties))]
[GenericTypeArguments(typeof(LargeStructWithProperties))]
[GenericTypeArguments(typeof(int))]
Copy link
Member

Choose a reason for hiding this comment

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

the code looks good to me, but I just wonder if it is a real use case: (de)serializing a single integer.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is there primarily to detect general front-end overhead of the serializer without any extra baggage.

However, one potential scenario is enumerating over a list of elements and calling serialize on each one.

However I don't recommend re-entering the serializer for each element -- first there is overhead of a dictionary lookup in the front-end to get the converter for each element and second because currently the "JsonPath" exception functionality doesn't work on re-entry (we need to add a new API to pass that state).

The better way is to write a custom converter for that collection which in turn would forward to a cached element converter. However, it may be that the list of element types are not known (base type of System.Object for example), so the dictionary lookup may need to occur per element anyway in order to find the converter for each element.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the explanation. If this provides value to you then I am merging it.

String2 = "2",
String3 = "3",
String4 = "4",
String5 = "5",
Copy link
Member

Choose a reason for hiding this comment

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

no initialization of the int properties?

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.

None yet

3 participants