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

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

Open
ROBYER1 opened this issue Apr 7, 2021 · 2 comments

Comments

@ROBYER1
Copy link

ROBYER1 commented Apr 7, 2021

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
Copy link
Owner

Bunny83 commented Apr 7, 2021

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
Copy link
Author

ROBYER1 commented Apr 7, 2021

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

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