Bunny83 / SimpleJSON

A simple JSON parser in C#
MIT License
735 stars 294 forks source link

JSONNode.IsNull always returns 'false' - working fine in another version of SimpleJSON? #45

Open ROBYER1 opened 3 years ago

ROBYER1 commented 3 years ago

Until now I was using this SimpleJSON script repo: https://github.com/HenrikPoulsen/SimpleJSON/blob/master/SimpleJSON.cs

Before switching over to trying your SimpleJSON scripts in a project as it looked better supported and also had a means of checking for keys in a JSONNode

When pulling a JSON in from the web, I was using a check to see if the JSON was null, this worked with their version, but when using yours I tried both jsonnode.IsNull which always returned false, or when using if(jsonnode == null) it would always be false too as your script gives me "\"\"" when reading an empty returned JSONNode.

Screenshot-2021-04-07-at-17 29 39 Screenshot 2021-04-07 at 18 28 59 Screenshot-2021-04-07-at-17 46 15

I am wondering if there is anything that can be done about this? I had to roll back to the HenrikPoulsen version linked above to get the null checks working again in our code, but their version is missing .key definitions when trying to use a helper script to check if a JSONNode contains a key.. so having this fixed would be great!

The JSON we receive is null effectively as when grabbing the value from Visual Studio and pasting it into a JSON reader online or VS Code, we have to remove all the \ and " speech marks from the string anyway so looks like the Henrik version has some more robust is JSON null checks?

Bunny83 commented 3 years ago

Well a json "null" value is a very specific value. In your case it seems that your input text is just an empty text. So by definition an empty string is not valid json and therefore the behaviour is undefined. The old implementation did not handle primitive json values as root element properly, the current implementation does handle those. So if you parse a string that contains:

null true "some text" it gets properly parsed which was essentially ignored in the old implementation. Technically the text should represent a "json value" which by definition can not be just empty as this would not resolve to anything. Of course the case of an empty string could resolve to JSONNull but it's not really more correct than the current implementation which simply represents an empty string (which the source actually is).

What we can do is simply replace the [last line in ParseElement](https://github.com/Bunny83/SimpleJSON/blob/master/SimpleJSON.cs#L538) with

return JSONNull.CreateOrGet();

This should return a JSONNull instance if the input string is empty. Alternatively you could return just null. Currently I don't have time to change that behaviour. Also as I said an empty string is not valid json. Just try validating an empty input in jsonlint and you will get an error. So the most "correct" behaviour would be to throw an exception. However I don't like exceptions as it makes the usage more difficult. If you don't want to change the behaviour of SimpleJSON, you just have to check for an empty result before you parse it.

ROBYER1 commented 3 years ago

Thanks for the detailed response and apologies if I at all made it sound like the linked repo was better in any sense as I appreciate the support you are giving this repo and the feature set!

I will try out your suggestions tomorrow when I get a moment and close this issue if the suggested line change works or at least checking for an empty result before I parse as a general practice for future projects reading JSON