Skip to content

Commit

Permalink
Updates the addAll methods to have optional wrapping.
Browse files Browse the repository at this point in the history
When called from the constructor, the individual items in the
collection/array are wrapped as done originally before the `putAll`
methods were added.

However this commit changes how `putAll` works. The items are no longer
wrapped in order to keep consistency with the other `put` methods.

However this can lead to inconsistencies with expectations. For example
code like this will create a mixed JSONArray, some items wrapped, others
not:

```java
SomeBean[] myArr = new SomeBean[]{ new SomeBean(1), new SomeBean(2) };
JSONArray jArr = new JSONArray(myArr); // these will be wrapped
// these will not be wrapped
jArr.putAll(new SomeBean[]{ new SomeBean(3), new SomeBean(4) });
```

For consistency, it would be recommended that the above code is changed
to look like 1 of 2 ways.

Option 1:
```Java
SomeBean[] myArr = new SomeBean[]{ new SomeBean(1), new SomeBean(2) };
JSONArray jArr = new JSONArray();
jArr.putAll(myArr); // will not be wrapped
// these will not be wrapped
jArr.putAll(new SomeBean[]{ new SomeBean(3), new SomeBean(4) });
// our jArr is now consistent.
```

Option 2:
```Java
SomeBean[] myArr = new SomeBean[]{ new SomeBean(1), new SomeBean(2) };
JSONArray jArr = new JSONArray(myArr); // these will be wrapped
// these will be wrapped
jArr.putAll(new JSONArray(new SomeBean[]{ new SomeBean(3), new
SomeBean(4) }));
// our jArr is now consistent.
```
  • Loading branch information
John J. Aylward committed Jul 30, 2020
1 parent eb774b3 commit 46cab0f
Showing 1 changed file with 46 additions and 18 deletions.
64 changes: 46 additions & 18 deletions src/main/java/org/json/JSONArray.java
Expand Up @@ -173,7 +173,7 @@ public JSONArray(Collection<?> collection) {
this.myArrayList = new ArrayList<Object>();
} else {
this.myArrayList = new ArrayList<Object>(collection.size());
this.addAll(collection);
this.addAll(collection, true);
}
}

Expand All @@ -188,7 +188,7 @@ public JSONArray(Iterable<?> iter) {
if (iter == null) {
return;
}
this.addAll(iter);
this.addAll(iter, true);
}

/**
Expand Down Expand Up @@ -225,7 +225,7 @@ public JSONArray(Object array) throws JSONException {
throw new JSONException(
"JSONArray initial value should be a string or collection or array.");
}
this.addAll(array);
this.addAll(array, true);
}

/**
Expand Down Expand Up @@ -1206,7 +1206,7 @@ public JSONArray put(int index, Object value) throws JSONException {
* @return this.
*/
public JSONArray putAll(Collection<?> collection) {
this.addAll(collection);
this.addAll(collection, false);
return this;
}

Expand All @@ -1218,7 +1218,7 @@ public JSONArray putAll(Collection<?> collection) {
* @return this.
*/
public JSONArray putAll(Iterable<?> iter) {
this.addAll(iter);
this.addAll(iter, false);
return this;
}

Expand Down Expand Up @@ -1250,7 +1250,7 @@ public JSONArray putAll(JSONArray array) {
* Thrown if the array parameter is null.
*/
public JSONArray putAll(Object array) throws JSONException {
this.addAll(array);
this.addAll(array, false);
return this;
}

Expand Down Expand Up @@ -1585,11 +1585,21 @@ public boolean isEmpty() {
*
* @param collection
* A Collection.
* @param wrap
* {@code true} to call {@link JSONObject#wrap(Object)} for each item,
* {@code false} to add the items directly
*
*/
private void addAll(Collection<?> collection) {
private void addAll(Collection<?> collection, boolean wrap) {
this.myArrayList.ensureCapacity(this.myArrayList.size() + collection.size());
for (Object o: collection){
this.myArrayList.add(JSONObject.wrap(o));
if (wrap) {
for (Object o: collection){
this.put(JSONObject.wrap(o));
}
} else {
for (Object o: collection){
this.put(o);
}
}
}

Expand All @@ -1598,10 +1608,19 @@ private void addAll(Collection<?> collection) {
*
* @param iter
* An Iterable.
*/
private void addAll(Iterable<?> iter) {
for (Object o: iter){
this.myArrayList.add(JSONObject.wrap(o));
* @param wrap
* {@code true} to call {@link JSONObject#wrap(Object)} for each item,
* {@code false} to add the items directly
*/
private void addAll(Iterable<?> iter, boolean wrap) {
if (wrap) {
for (Object o: iter){
this.put(JSONObject.wrap(o));
}
} else {
for (Object o: iter){
this.put(o);
}
}
}

Expand All @@ -1612,28 +1631,37 @@ private void addAll(Iterable<?> iter) {
* Array. If the parameter passed is null, or not an array,
* JSONArray, Collection, or Iterable, an exception will be
* thrown.
* @param wrap
* {@code true} to call {@link JSONObject#wrap(Object)} for each item,
* {@code false} to add the items directly
*
* @throws JSONException
* If not an array or if an array value is non-finite number.
* @throws NullPointerException
* Thrown if the array parameter is null.
*/
private void addAll(Object array) throws JSONException {
private void addAll(Object array, boolean wrap) throws JSONException {
if (array.getClass().isArray()) {
int length = Array.getLength(array);
this.myArrayList.ensureCapacity(this.myArrayList.size() + length);
for (int i = 0; i < length; i += 1) {
this.put(JSONObject.wrap(Array.get(array, i)));
if (wrap) {
for (int i = 0; i < length; i += 1) {
this.put(JSONObject.wrap(Array.get(array, i)));
}
} else {
for (int i = 0; i < length; i += 1) {
this.put(Array.get(array, i));
}
}
} else if (array instanceof JSONArray) {
// use the built in array list `addAll` as all object
// wrapping should have been completed in the original
// JSONArray
this.myArrayList.addAll(((JSONArray)array).myArrayList);
} else if (array instanceof Collection) {
this.addAll((Collection<?>)array);
this.addAll((Collection<?>)array, wrap);
} else if (array instanceof Iterable) {
this.addAll((Iterable<?>)array);
this.addAll((Iterable<?>)array, wrap);
} else {
throw new JSONException(
"JSONArray initial value should be a string or collection or array.");
Expand Down

0 comments on commit 46cab0f

Please sign in to comment.