RFR: json patches introduced to the OpenJDK copy in JDK-8246435

[Magnus Ihse Bursie]

On Wed, 3 Jun 2020 12:42:45 GMT, Erik Helin <ehelin 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.
>
> 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).

To me there's no deeper semantic difference between "append" and "concat". To be precise, this method do a flattening
append/concat, and should probably be named something like addAndFlatten to be completely unambiguous. But that kind of
naming did not seem to fit in with the rest of the design here.

If you insist that "concat" means "flatten while adding" but "append" does not, sure, I can rename it.

> 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)`.

I don't care about a non-flattening constructor. You can add one if you need one. But I can change it to a static
factory method instead, if you want. It's probably clearer, given that we can agree on a suitable name. (I'll accept
"concat" if you insist, but I'm not persuaded that it's really better...)

-------------

PR: https://git.openjdk.java.net/skara/pull/636