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 support for specifying comparators in set ops #1367

Closed
wants to merge 1 commit into from

Conversation

rcoh
Copy link

@rcoh rcoh commented Dec 4, 2013

This commit adds support for specifying comparators in union,
difference, and intersection (#1339). The specification of comparators allows
for library users to specify _.isEqual for deep equality or to
specify their own methods that match a subset of paramters. It
also adds deepUnion, deepIntersection and deepDifference functions,
convienience functions that specify _.isEqual.

Under the hood, it actually allows the specification of a "contains"
function into several relevant relevant functions. This is created from
the comparator with a closure. The code to allow this a little cumbersome.
I'm open to suggestions on better methods of passing through a containment-
checking function -- one possibility would be to store the containment
function of the moment on this.

Let me know your temperature for merging these changes.

This commit adds support for specifying comparators in union,
difference, and intersection. The specification of comparators allows
for library users to specify _.isEqual for deep equality or to
specify their own methods that match a subset of paramters. It
also adds deepUnion, deepIntersection and deepDifference functions,
convienience functions that specify _.isEqual.

Under the hood, it actually allows the specification of a "contains"
function into several relevant relevant functions. This is created from
the comparator with a closure. The code to allow this a little cumbersome.
I'm open to suggestions on better methods of passing through a containment-
checking function -- one possibility would be to store the containment
function of the moment on this.

Let me know your temperature for merging these changes.
@jbrains
Copy link

jbrains commented Dec 15, 2013

+1

I'm curious: why compare stringified objects tests, rather than simply compare the resulting objects?

@rcoh
Copy link
Author

rcoh commented Dec 16, 2013

Following the pattern of existing tests. I assume it's to avoid relying on the correctness of deep equals

@jbrains
Copy link

jbrains commented Dec 16, 2013

I see. It seems risky to do it that way, but I understand the desire to write the assertions concisely. I'd probably write them out "the long way", myself. :)

Thanks again.

@jashkenas
Copy link
Owner

Very lovely stuff — but probably just slightly too esoteric to ship in core Underscore. I'd recommend moving this patch over to Underscore-contrib.

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.

None yet

3 participants