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

Converting strings to other types can change values #69

Closed
yaccob opened this issue Nov 14, 2012 · 3 comments
Closed

Converting strings to other types can change values #69

yaccob opened this issue Nov 14, 2012 · 3 comments

Comments

@yaccob
Copy link
Contributor

yaccob commented Nov 14, 2012

Hi Douglas

The attempt to guess the type of a text node or an attribute value may change the type which may be unintended. If the intended type is a string, "01" is not equal with "1". So operations change values which can cause unwanted effects.
Furthermore the behavior is not consistent: "00" remains unchanged (remains a String) while "01" is converted to the number 1 as you can see in the sample below.
Also automatic conversion to boolean might have unwanted effects. Imagine the title of a book's chapter is "True" - intentionally with upper case "T". As far as I could see currently I am not able to convert this title to Json without implicitly changing it to a boolean and therefore losing information.

Sample:

xml:
<root>
  <id>01</id>
  <id>1</id>
  <id>00</id>
  <id>0</id>
  <title>True</title>
</root>

code:
println org.json.JSONML.toJSONArray(xml)

output:
["root",["id",1],["id",1],["id","00"],["id",0],["title",true]]

I would suggest to either change this or at least to add an option to control the behavior in a static way. (e.g. adding a non-final static boolean field KEEP_STRINGS which can be set to true)

Best regards,
yaccob

@douglascrockford
Copy link
Contributor

You have the source.

@uklimaschewski
Copy link

@yaccob adding a non-final static boolean field KEEP_STRINGS which can be set to true

This will make toJSONArray() not thread-safe. Better add another boolean parameter to the parse() method and introduce new toXXX() methods with this parameter.

@yaccob
Copy link
Contributor Author

yaccob commented Nov 14, 2012

@uklimaschewski introduce new toXXX() methods
Agreed, introducing new toXXX() methods is anyway much cleaner. I'll simply overload the parse method and add a new toJsonML(String) and a new toJsonML(XMLTokener) method using the overloaded parse method. Thanks for your input.

BGehrels pushed a commit to BGehrels/JSON-java that referenced this issue Apr 29, 2020
Couple of the files were difficult to identify the changes, but it was worth it to add the new tests. Thanks!!!
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

3 participants