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 Map and Set support to deepEqual() #254

Merged
merged 7 commits into from Nov 1, 2018

Conversation

@kanongil
Copy link
Member

kanongil commented May 20, 2018

In catbox, I have a need to add tests that compares the Map and Set types, so I had a look at adding it to Hoek. This also ties in nicely with #248, and allows the tests in it to actually work….

Regarding the implementation, I had to define what a deep-equal on a Set and Map really means. What I came up with is:

Set is deep equal when…

  • Both have same number of elements.
  • All elements in obj have a deepEqual() matching element in ref.
  • Either all members are also in the reference using a .has() test, or an ordered test for deep equality matches for all members.

I need to use the order of the elements before a deep comparison can be done. Otherwise, comparing new Set({}, {}), new Set({}, 1), and new Set({}) would not make much sense.

Map is deep equal when…

  • Both have same number of elements.
  • All keys are in both maps.
  • The values of a key are deep equal.

For this, I haven't included an ordered comparison. This makes keys special, since they are only compared using .has(). I can add it if you think it matters.

The implementation is based on the rework in #242.

@kanongil

This comment has been minimized.

Copy link
Member Author

kanongil commented May 22, 2018

Hmm, a full sweep for deep Set comparisons could probably be implemented somewhat reasonably by matching against a copy of the reference set, where matches are removed. I will have a look at that.

FYI, I'm already using this branch to support my new addon here: https://github.com/kanongil/granola.

@kanongil kanongil force-pushed the kanongil:map-set-deepequal branch from 5b06351 to c522908 May 29, 2018
@kanongil

This comment has been minimized.

Copy link
Member Author

kanongil commented May 29, 2018

Updated to check all elements for deep Set comparisons.

The implementation has a fast path for comparing sets that only contain simple types and/or similar object references. If this fails, an exhaustive search is performed, optimized for ordered elements.

@hueniverse hueniverse merged commit 525ff44 into hapijs:master Nov 1, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hueniverse hueniverse self-assigned this Nov 1, 2018
@hueniverse hueniverse added this to the 6.0.0 milestone Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.