JSON-java icon indicating copy to clipboard operation
JSON-java copied to clipboard

[RFC] Implement custom iterator for JSONArray to ensure fetched values are JSON compliant

Open johnjaylward opened this issue 3 years ago • 3 comments

This has been a "known" issue for some time, but as mentioned in #672, the JSONArray.iterator method is exposing the raw values from the backing array instead of JSON-compliant values.

We should do 1 of 2 things:

  1. Implement a private/internal JSONArrayIterator class that would ensure any fetched values are properly "wrapped" and thus suitable output to JSON processors.
  2. Update all of our put* operations to ensure that all values are properly "wrapped" in both JSONObject and JSONArray. This would also allow us to simplify our toString methods and remove the wrapping from there as all values should be proper JSON.

Option 1 keeps the wrapping "lazy" and thus would only guarantee JSON compliance on values that are fetched. Fetching a non-wrapped value multiple times would cause wrapping to happen each time.

Option 2 makes the wrapping more proactive; guaranteeing that our JSON compliance happens up-front even if the value is not fetched later. Fetching an initially non-wrapped value multiple times would cause NO wrapping to happen at fetch as it would have been pre-wrapped by the put operation.

Both options only affect code-built JSONObjects/Arrays. Parsed objects would be unaffected as the parser is only placing JSON-compliant values in the backing collections.

Thoughts on whether we should implement either option, something else, or neither?

johnjaylward avatar Mar 21 '22 16:03 johnjaylward

Option 1 sounds easier/more straightforward. My concern is whether this might change behavior in a way that could break someone's existing code. If so, then not all issues have to be fixed. But if you think I am missing something in your proposal, let me know.

stleary avatar Mar 22 '22 02:03 stleary

I agree it's possible it's a breaking change, but the way the put methods work today, my guess is that fixing this would likely simplify any code that is using it. My thought is that they wouldn't need as much conditional handling (i.e. am I going to get a Map or a JSONObject this time?)

My guess is that this would fix more code than it hurts, but that's just based on how I use the library.

johnjaylward avatar Mar 22 '22 02:03 johnjaylward

@johnjaylward Do you have any suggestions for how to proceed with this issue?

stleary avatar Sep 30 '23 04:09 stleary