RFR: json patches introduced to the OpenJDK copy in JDK-8246435
Erik Helin
ehelin at openjdk.java.net
Wed Jun 3 12:47:26 UTC 2020
On Wed, 3 Jun 2020 12:16:36 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:
> This PR contains the proposed changes to the json system made in JDK-8246435. If the same changes are made in Skara,
> tthe two code bases are kept from diverging.
Hi Magnus,
thanks for contributing! Looks really nice in general, I just had two small comments about the changes to `JSONArray`.
Thanks!
Erik
json/src/main/java/org/openjdk/skara/json/JSONArray.java line 42:
> 41:
> 42: private void append(JSONValue value) {
> 43: if (value instanceof JSONArray) {
Could you rename this to `concat` and explicitly take a `JSONArray` instead as parameter? I'm fine with having
`append`, but `append` should be just `append` (that is, append a single item to the array).
json/src/main/java/org/openjdk/skara/json/JSONArray.java line 52:
> 51:
> 52: public JSONArray(JSONValue value, JSONValue... values) {
> 53: this.values = new ArrayList<JSONValue>(values.length + 1);
Related to the comment about `concat` vs `append`, I would prefer that the constructor does `append` and not `concat`.
One could imagine a `static` version of `concat` as well, e.g. `public static JSONArray concat(JSONArray... arrays)`.
-------------
Changes requested by ehelin (Reviewer).
PR: https://git.openjdk.java.net/skara/pull/636
More information about the skara-dev
mailing list