Investigate issues with changing only the JSONObject and JSONArray ctors

[stleary]

https://github.com/douglascrockford/JSON-java/pull/153 was closed with the intent that only the JSONObject and JSONArray ctors might be changed as folllows:

public JSONObject(Map<String, ?> map) {
    this.map = new HashMap<String, Object>();
    if (map != null) {
        for (final Entry<String, ?> e : map.entrySet()) {
            final Object value = e.getValue();
            if (value != null) {
                this.map.put(e.getKey(), wrap(value));
            }
        }
    }
}

public JSONArray(Collection<?> collection) {
    this.myArrayList = new ArrayList<Object>();
    if (collection != null) {
        for (Object o : collection) {
            this.myArrayList.add(JSONObject.wrap(o));
        }
    }
}

These limited changes pass unit tests. Need to add tests at least for Map<String, String> and Collection<String>. Also, if this is the only change made, are there any use cases where users' code could break or not work as intended? In particular, check the put(), opt(), and get() methods.

[treyerl]

import java.util.HashMap;
import java.util.Map;

import org.json.JSONObject;

public class Tester {

    public static void main(String[] args) {
        // if only the constructors and not the wrap() method are being changed
        // then will case1 work? --> YES
        Map<String, Map<String, String>> case1 = new HashMap<String, Map<String, String>>();
        Map<String, String> sub = new HashMap<String, String>();
        sub.put("one", "ONE");
        case1.put("1", sub);
        JSONObject j1 = new JSONObject(case1);

        // If the wrap method is being adapted then a JSONObject's map can hold nested maps.
        // Will the writeValue() method be affected and not work properly anymore? --> NO
        System.out.println(j1); // assert equals to "{"1":{"one":"ONE"}}"

        // JSONObject.valueToString() is just another method calling JSONObject(Map<?, ?> map)

    }
}

Why are you trying to cut away necessary changes? #153 is as minimal as possible already. Don’t you see that @douglascrockford with the commit Java1.8 a9a076 had a wrong concept of super types in generics. To me it looks like as he thought the root super type (super type of all kinds of a Map) of Map<String, String> would be Map<String, Object>. But in generic programming the root super type is Map<?,?>. #153 corrects this wherever the root super type of a generic type is needed. Therefore we would like to change also the put(Map<String,Object> map) methods of JSONObject and JSONArray to put(Map<?,?> map). There is really no need to stick to the constructors only - unless you want to discuss this in another pull request next week and the week after and again next year. People have been pointing at it already last winter. I bet they have their own version of the library now, or stick with an old pre Java 1.8 version of this library. In my project I don’t use the put(map) methods. But others probably do. And put(Map<String, Object> map) is not backwards compatible. Only put(Map<?,?> map) is.

So, if you change only the ctors as proposed here, my project will be able to use the latest version of JSON-java. If that’s an acceptable minimum for you, fine. But as you have been asking for higher standards, I think it would be appropriate look at #153 as a solution for a misconception in a9a076 regarding super types in generic programming.

[stleary]

What you say has merit, and I agree that the proposed change is more correct than the current implementation. The ctor change is only being considered because there is a compelling case that some apps were broken by the upgrade to 20150729. Otherwise, the stability of the API has higher priority than making the code more correct.

[stleary]

Decision was to go with the solution in https://github.com/douglascrockford/JSON-java/pull/153.