JSON-java
JSON-java copied to clipboard
stleary
JSONTokener should implement java.io.Closeable
While JSONTokener(java.io.Reader) and JSONTokener(java.io.InputStream) constructors have explicit notes about JSONTokener not closing the reader/stream, I’d like to have it close the reader/stream. Implementing java.io.Closeable isn’t backward-incompatible (if the library user doesn’t explicitly close it, nothing happens), and for those who use try-with-resources construct, having to keep a separate reference for the reader/stream is a pain. Although try-with-resources itself is Java 7+, the java.io.Closeable interface is @since 1.5, so implementing the interface can be done without losing Java 6 compatibility.
Its completed & reviewed at commit https://github.com/stleary/JSON-java/commit/e1eabc9c27f954ce8fe8032f12f92f51c0e7c9eb
Closed due to lack of activity. Please post here if you think it should be reopened.
Should I spam you with “what’s going on?” messages every few weeks? Doing so would just waste time of both of us. Lack of activity is not the same as lack of interest or usefulness. Please don’t close the issue until it gets implemented and released; especially not as “completed”, which is simply not true.
@jtotht Thanks for the feedback. Historically, issues have been left open on this repo for an extended period, which has led to its own set of problems. I am making an effort to better manage the project by closing issues that are unlikely to be acted upon in the near term. The label "completed" that GitHub applies when an issue is closed with a comment is just GitHub behavior and does not necessarily mean that the requested feature or fix has been implemented. Given that the project is now primarily in maintenance mode, I am cautious about making major changes to the API, such as making JSONTokener implement Closeable. A change of this magnitude requires a compelling use case, given its potential impact on backward compatibility and API complexity. Please feel free to make that case if you wish.
I am making an effort to better manage the project by closing issues that are unlikely to be acted upon in the near term.
For things that may once be fixed/implemented, but unlikely to happen in the near term, you could introduce a label that makes this situation clear and allows hiding them in the issues list (example: hide in-progress issues) without closing the issue.
The label "completed" that GitHub applies when an issue is closed with a comment is just GitHub behavior and does not necessarily mean that the requested feature or fix has been implemented.
Recently (a few months ago, if I remember correctly) GitHub introduced a new feature allowing closing issues as “not planned”. With this new feature, the default of closing as “completed” does communicate that you believe that the requested feature has been implemented.
A change of this magnitude requires a compelling use case […]. Please feel free to make that case if you wish.
In my servlet application, I use JSON.org to parse JSON request bodies sent by the frontend. I do so by constructing JSONTokener objects using the return value of HttpServletRequest#getReader(). Since this reader needs to be closed once it’s no longer used, I have to pass both the Reader object and the JSONTokener object along (I need to pass them along because there are two methods – one for reading arrays and another one for reading objects –, but the extraction from request and error handling parts are common). Is this compelling enough considering the magnitude of this change? (Which isn’t that large IMO.)
given its potential impact on backward compatibility and API complexity
As I stated in the description, it’d be 100% backward-compatible: if one doesn’t call JSONTokener#close() and doesn’t use JSONTokener objects in try-with-resources constructs (neither of which is currently possible, so no existing users do so), nothing changes for them. It does make the API a bit more complex, but I believe that the benefits of being able to use the tokeners in try-with-resources constructs outweighs the costs of the API complexity increase.
It is not 100% backwards compatible. Adding the closable api makes integration with the android namespace implementation harder.
Code that uses the new iclosable would not be dropin to android, and that would need to be discussed.
