JSONWriter uses Appendable (v2) - #259
Conversation
Sorry, something went wrong.
|
This is a much more straightforward change. The tests from the previous test cases apply. |
Sorry, something went wrong.
|
This looks simple enough. Your other pull request did have other good changes in it as well. Maybe if you find some good places to split the functional changes into separate pull requests it will be easier to get them approved. I would especially like to see consistency in how the JSONString interface is used within the library, so if you could open a pull request (or at least an issue) for that, I would appreciate it. It should note where the inconsistencies happen and the current way the interface is used at each point. We can then discuss the appropriate way that the interface should be handled from there. |
Sorry, something went wrong.
|
Looks good, will leave the pull request open for a few days for comments. |
Sorry, something went wrong.
What problem does this code solve?
java.lang.Appendable instead. Situations where Appendable
can be used where Writer cannot include the StringBuffer,
StringBuilder, and CharBuffer classes.
Risks
Low risk. This is modifying an API method to take an interface instead of a base class, which is generally a good practice. The positive unit test coverage is good. But JSONWriter is lacking coverage for most of the thrown exceptions, including in modified methods. OK to commit, but better unit test code coverage will be required before this code can be included in a release. See stleary/JSON-Java-unit-test#56.
Changes to the API?
The constructor for JSONWriter has changed from Writer to Appendable.
Will this require a new release?
Can be rolled into the next release, pending completion of the unit tests.
Should the documentation be updated?
Not required.
Does it break the unit tests?
No, the current tests run successfully. However, new unit tests have been added. See
stleary/JSON-Java-unit-test#53