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

Add optJSONArray method to JSONObject with a default value #773

Merged
merged 2 commits into from
Sep 30, 2023

Conversation

eedijs
Copy link
Contributor

@eedijs eedijs commented Sep 25, 2023

The JSONObject class had a missing optJSONArray method with a default return value, which is very useful in some cases, so you don't have to write code like this:

JSONArray jsonArray = jsonObject.has("key") ? jsonObject.getJSONArray("key") : new JSONArray;

And instead just write:

JSONArray jsonArray = jsonObject.optJSONArray("key", new JSONArray());

@stleary
Copy link
Owner

stleary commented Sep 26, 2023

@eedijs Thanks, agree it should be possible to return a default value. Is there a reason why this method was not also added to JSONArray?

@eedijs
Copy link
Contributor Author

eedijs commented Sep 27, 2023

@eedijs Thanks, agree it should be possible to return a default value. Is there a reason why this method was not also added to JSONArray?

Sorry, didn't even check the JSONArray class. Seems like it is also missing the optJSONObject method with a default value.

I will go over both JSONObject and JSONArray classes once more and see if anything else need an opt method with default values.

@stleary
Copy link
Owner

stleary commented Sep 27, 2023

I think there's a good case to be made for optJSONArray() with default. Probably not necessary to do an exhaustive review of all of the opt methods.

@eedijs
Copy link
Contributor Author

eedijs commented Sep 27, 2023

I think there's a good case to be made for optJSONArray() with default. Probably not necessary to do an exhaustive review of all of the opt methods.

Ok, I just commited optJSONArray and optJSONObject methods for the JSONArray class.

@stleary
Copy link
Owner

stleary commented Sep 28, 2023

What problem does this code solve?
Opt methods for JSONArray and JSONObject should allow a default value to be returned.

Risks
Low

Changes to the API?
Yes, new API methods are added where a default parameter is included. The default param is the same type as the return value.

Will this require a new release?
No

Should the documentation be updated?
No

Does it break the unit tests?
No. New unit tests were added for the new API methods.

Was any code refactored in this commit?
No

Review status
APPROVED

Starting 3-day comment window

@stleary stleary merged commit ef68cdf into stleary:master Sep 30, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants