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.

Sign up for GitHub

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

Jump to bottom

Added JsonObject method isEmpty() #2233

Merged
merged 3 commits into from
Nov 13, 2022
Merged

Added JsonObject method isEmpty() #2233

merged 3 commits into from
Nov 13, 2022

Conversation

dhoard
Copy link
Contributor

@dhoard dhoard commented Nov 4, 2022

Code change

Added JsonObject method isEmpty().

Rationale

There are scenarios where there is a need to check whether a JsonObject contains any key/value pairs.

This mirrors the existing functionality of JsonArray.isEmpty()

Marcono1234
Marcono1234 reviewed Nov 4, 2022
View reviewed changes
Copy link
Collaborator

@Marcono1234 Marcono1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this pull request!

Could you please for completeness sake also add a small test to JsonObjectTest?

Your argument about consistency with JsonArray.isEmpty() is reasonable, but on the other hand the question might be how many Map convenience methods we want to add, see also related #1667 (comment).

gson/src/main/java/com/google/gson/JsonObject.java Outdated Show resolved Hide resolved
gson/src/main/java/com/google/gson/JsonObject.java Show resolved Hide resolved
@dhoard dhoard force-pushed the jsonobject-is-empty branch from 268cee7 to ec19d71 Compare November 6, 2022 14:18
@dhoard
Copy link
Contributor Author

dhoard commented Nov 6, 2022

Your argument about consistency with JsonArray.isEmpty() is reasonable, but on the other hand the question might be how many Map convenience methods we want to add, see also related #1667 (comment).

I feel that isEmpty() and clear() ( mentioned in #1667 (comment) ) make sense because they are around the object's internal state.

Other Map methods are around conversion, which I see as a different aspect, and agree they can become unwieldy.

@dhoard dhoard force-pushed the jsonobject-is-empty branch from ec19d71 to 90db072 Compare November 6, 2022 15:31
mariocandela
mariocandela reviewed Nov 9, 2022
View reviewed changes
Copy link

@mariocandela mariocandela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about it ?

gson/src/test/java/com/google/gson/JsonObjectTest.java Outdated Show resolved Hide resolved
gson/src/test/java/com/google/gson/JsonObjectTest.java Outdated Show resolved Hide resolved
gson/src/test/java/com/google/gson/JsonObjectTest.java Outdated Show resolved Hide resolved
@eamonnmcmanus
Copy link
Member

Given that we already have both JsonArray.isEmpty() and JsonObject.size(), I agree that this method makes sense for consistency. Additionally, it appears that most of Google's internal uses of JsonObject.size() in non-test code are comparing the size against 0.

@eamonnmcmanus eamonnmcmanus merged commit ceb3b87 into google:master Nov 13, 2022
tibor-universe pushed a commit to getuniverse/gson that referenced this pull request Jan 20, 2023
@dhoard @tibor-universe
Added JsonObject method isEmpty() (google#2233)
6b29f04 
* Added isEmpty()

* Fixed Javadoc typo

* Changed test to use assertTrue() and assertFalse()

(cherry picked from commit ceb3b87)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Marcono1234 Marcono1234 Marcono1234 left review comments

@mariocandela mariocandela mariocandela approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dhoard @eamonnmcmanus @Marcono1234 @mariocandela