Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor - JSONObject.put(String Object) throws wrong Exception #402

Closed
krakenreleased opened this issue Feb 22, 2018 · 4 comments
Closed

Minor - JSONObject.put(String Object) throws wrong Exception #402

krakenreleased opened this issue Feb 22, 2018 · 4 comments

Comments

@krakenreleased
Copy link

krakenreleased commented Feb 22, 2018

The javadoc states, invoking JSONObject.put(String key, Object value) with a null key should throw a JSONException, however it is throwing a null pointer exception instead.

     * Put a key/value pair in the JSONObject. If the value is null, then the
     * key will be removed from the JSONObject if it is present.
     *
     * @param key
     *            A key string.
     * @param value
     *            An object which is the value. It should be of one of these
     *            types: Boolean, Double, Integer, JSONArray, JSONObject, Long,
     *            String, or the JSONObject.NULL object.
     * @return this.
     * @throws JSONException
     *             If the value is non-finite number or if the key is null.
     */
    public JSONObject put(String key, Object value) throws JSONException {
        if (key == null) {
            throw new NullPointerException("Null key.");
        }
        if (value != null) {
            testValidity(value);
            this.map.put(key, value);
        } else {
            this.remove(key);
        }
        return this;
    }
@johnjaylward
Copy link
Contributor

I will have a patch for this soon.

@johnjaylward
Copy link
Contributor

Actually, @stleary, Douglas was the last one to touch both the JavaDoc and the code that throws the exception. He changed it way back in a73066f from throwing a JSONException to throwing the NPE.

I'd say we defer to the JavaDoc on this one, but perhaps a simple JavaDoc update is all that's required. Sadly the commit message only really mentions one of many changes that were applied (mostly formatting), so I'm not sure if the code change was intended or not,

@stleary
Copy link
Owner

stleary commented Feb 27, 2018

@johnjaylward Thanks for looking up the history. No preference. Leaving the NullPointerException alone is safer, changing to JSONException maintains consistency with the most of the rest of the lib, other than JSONPointer.

@stleary
Copy link
Owner

stleary commented Mar 17, 2018

I believe this has been addressed by #405. If you think otherwise, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants