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

equals method #33

Closed
wants to merge 2 commits into from
Closed

equals method #33

wants to merge 2 commits into from

Conversation

fixje
Copy link

@fixje fixje commented Nov 9, 2011

I have implemented equals methods for JSONObject and JSONArray in order to achieve duplicate-elimination in Sets. It is just a first step, beacuse there is still a hashCode() method missing. I didn't find an nice way to realize that for now.

Hope I could help

Best Regards,
Markus

@douglascrockford
Copy link
Contributor

What happens if a JSON object contains a cycle?
What happens if the other object contains an extra property?

@fixje
Copy link
Author

fixje commented Nov 9, 2011

What happens if the other object contains an extra property?
dumb mistake of mine... additionally checking the if the length of both keysets (respectively length of both arrays) is the same solves this

What happens if a JSON object contains a cycle?
There is no general solution for this issue if you consider equality as the equality of referenced objects.
One could make the same assumption like for toString()
"Warning: This method assumes that the data structure is acyclical."
But equals() is too often used implicitly, such that making this assumption unavoidably leads to problems.
In fact, it doesn't make sense to go on in this direction.

BGehrels pushed a commit to BGehrels/JSON-java that referenced this pull request Apr 29, 2020
Fix some todos, clean up some tests, improve coverage
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

Successfully merging this pull request may close these issues.

2 participants