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