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

Can't read Json with a list called "Sets" - Error "Unexpected character encountered while parsing value" #2592

Closed
MatthiasBae opened this issue Oct 7, 2021 · 6 comments

Comments

@MatthiasBae
Copy link

Hello all,

I have a problem with Populating a class from a Json. Before this, I created this JSON without any problems with the exact same class i want to re-populate.

The error I get is: Newtonsoft.Json.JsonReaderException: "Unexpected character encountered while parsing value: [. Path 'Sessions[0].Workouts[0].Excercises[0].Sets', line 20, position 23."

I have a list of classes grafik
and it seems that the reader has a problem with the word "Sets" as list variable name. When i rename it to "Setz" or "Setss" or something else, everything works fine.
This is my JSON
grafik

This is the part of the class
grafik

The code fails here
grafik

@elgonzo
Copy link

elgonzo commented Oct 7, 2021

This looks strange. The information in your issue report does not give a hint aside from a supposed mismatch between the Json data structure and the C# object graph you try to deserialize into as indicated by the exception.

Can you provide a self-contained code example that reproduces the problem and can be simply copy-pasted into dotnetfiddle or a console project in VS? Because, I did a quick'n'dirty code test on dotnetfiddle (https://dotnetfiddle.net/vPOP2r), and i cannot reproduce...

@MatthiasBae
Copy link
Author

@elgonzo thanks for your reply. There should be no chance of a mismatch because the JSON got created with the same class a minute before I try to populate the class again

Attached you will find a VS Console Project for testing. The Json will be saved in the "..Debug\net5.0\Testfolder\2021-10-07.json"

JsonProblem Test.zip

@elgonzo
Copy link

elgonzo commented Oct 7, 2021

Alright. I know what causes the problem, but i do not really know why... (as of now).

Your TrainingExercise class has a constructor with an int sets argument. Changing the name of this constructor argument (for example to int numberOfSets) will solve the issue. Changing the name "Sets" of the public field to something else will not solve the issue, however. (In your report you mentioned the field variable "Sets" which i assumed you were referring to the field name "Sets" itself not the constructor argument name. But maybe i was misunderstanding your explanation there...)

This is how i imagine the problem manifests: As Newtonsoft.Json has to create TrainingExercise instances, it has to invoke the constructor with values for each constructor argument. It tries to do so by mapping each constructor argument with the value of a Json property (of the respective Json object) with a (case-insensitive) matching name. According to the constructor argument "sets", Newtonsoft.Json looks in the TrainingExercise Json object for an the "Sets" property, and finds it. And then proceeds to attempting to read/parse an integer value, but finds a Json array instead.


EDIT: Ignore the paragraphs below. There is a rather easy answer to my confusion, given in my follow-up comment below :-)

But there is a "But". I big one... I said "this is how imagine" with a reason. Which totally confuses me, and which is also why i wrote in the beginning that i don't know why it happens.

When reading your report, i already had a hunch it might be the constructor argument int sets. That's why i included a constructor with a int sets argument in my dotnetfiddle as well. But my dotnetfiddle does not fail, which is why i discarded my hunch. And this is also why i do not entirely trust my imagination here. I still have no clue of why your code is throwing when the argument name in the TrainingExercise constructor is int sets whereas my dotnetfiddle is not throwing. Hmm, puzzling... perhaps it might require a certain depth of nested Json objects to be triggered, perhaps it might be something else. As of now i have no solid idea, though...

@elgonzo
Copy link

elgonzo commented Oct 7, 2021

Update to my last post: I am a dumbo. It's obvious why my dotnetfiddle does not throw despite the constructor argument.

In my original dotnetfiddle, my code itself creates the JsonModel instance and passes it to the JsonConvert.PopulateObject method, thus Newtonsoft.Json won't create another one (and thus does not invoke the constructor, obviously.). If i alter my dotnetfiddle so that Newtonsoft.Json actually has to create a JsonModel instance, then my dotnetfiddle throws in the same manner as your code: https://dotnetfiddle.net/cGlouL

I am an idiot :-)

@MatthiasBae
Copy link
Author

@elgonzo Yeah I also had thought about the conflict between the "int sets" in the constructor and my List Sets... but I always thought it would be case sensitive and constructor variables would be ignored. (But I do not know because Iam new to the newtonsoft library). Changing the name of the List Sets only helps if you delete the old Json before executing the code and create a new one with the new list name.

But my question is now: Is it correct that it throws an exception or is it not? Because I would not expect this behavior because of a constructur parameter.

@elgonzo
Copy link

elgonzo commented Oct 7, 2021

I would argue that this is intended -- or perhaps better: established -- behavior. While the documentation is sparse, the documentation for the ConstructorHandling setting explains the constructor selection process. While the behavior with regard to parameterized constructors is not explained explicitly (or i couldn't find any such documentation), i see both logic and reason in the observed behavior. An int is not an array, and trying to map Json object properties to constructor arguments by matching names appears to be a sensible convention.

To do the constructor argument name-matching in a case-insensitive manner is also sensible[*]. Name-matching of field/property names during deserialization is also case-insensitive, so it would be inconsistent and potentially confusing to a user if confronted with a name-matching logic for constructor arguments that is different from the name-matching logic for fields/properties. (I am not saying that case-sensitive matching is not useful. In certain situations a user might prefer case-sensitive matching. Alas, despite being asked for such a feature, the author of Newtonsoft.Json never implemented an option for case-sensitive matching, as far as i know. :-( )

Fllowing this sentiment further, it seems reasonable and consistent that Newtonsoft.Json throws an exception in your case here when the constructor argument type is not compatible with the type of the json value being deserialized. Consistent, because it would also throw the same exception if your TrainingExercise were only have an (implicit) default constructor or a parameter-less constructor with the Sets field being an integer, like:

class TrainingExercise
{
    public string Name = "";
    public string Muscle = "";
    public double MaxWeight = 0;
    public double MinWeight = 0;

    public int Sets = 0;
} 

To me, the observed behavior with regard to your code/problem is both meaningful (as it makes sense on a logical/rational level) and consistent when compared with Newtonsoft.Json's behavior of mapping json object properties to fields/properties of C# classes.


[*] As a long-ish side note, case-insensitive matching deals nicely with the relatively common case of immutable types that have pascal-case properties with public getters, but no or private setters, and a public parameterized constructor with lower-case/camel-case argument names. The pascal-case property getters participate in serialization, leading to the json property names being identical to the C# property names. However, during deserialization of such immutable types, the parameterized constructor is involved with the lower/camel-case argument names. Having case-sensitive matching by default would require the user to either violate the most common C# naming convention, annotate the model classe(s) with attributes denoting the mapping between json property names and C# field/property/contructor argument names of immutable types, or use a custom contract resolver to realize that mapping, neither of which sounds very appealing to me :)

@MatthiasBae MatthiasBae closed this as not planned Won't fix, can't repro, duplicate, stale Jul 24, 2022
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

No branches or pull requests

2 participants