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 R.containsBy #1353

Closed
wants to merge 1 commit into from
Closed

Conversation

davidchambers
Copy link
Member

See #1349

R.containsWith is usually used in conjunction with a function such as (a, b) => a.x === b.x or (a, b) => f(a) === f(b) (occurrences on GitHub). R.containsBy would be more convenient in such cases.

In the rare cases where the symmetry of R.containsBy is undesirable, one can always use R.any. It's thus unnecessary to retain R.containsWith.


describe('containsBy', function() {

it('determines whether a projected list contains a projected value', function() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Is "projected" the right word? I'm trying to suggest the mapping of a domain to a codomain.

Copy link
Member

Choose a reason for hiding this comment

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

I think "projected" is perfect.

@TheLudd
Copy link
Contributor

TheLudd commented Aug 25, 2015

I have always found containsWith very confusing. This is perhaps somewhat better but what I would have expected is something like this (a -> Boolean) -> [a] -> Boolean. I.e. the first argument is a predicate function. If it returns true for any element in the list then this function returns true, otherwise false.

@davidchambers
Copy link
Member Author

This is perhaps somewhat better but what I would have expected is something like this (a -> Boolean) -> [a] -> Boolean. I.e. the first argument is a predicate function. If it returns true for any element in the list then this function returns true, otherwise false.

You're precisely defined R.any. ;)

I'm happy to deprecate R.containsWith without introducing R.containsBy. I'm eager to remove a confusing function, and I'm willing to add R.containsBy if necessary to achieve this.

@TheLudd
Copy link
Contributor

TheLudd commented Aug 25, 2015

You're precisely defined R.any. ;)

Oh, didn't realize that. Must be because I want to use contains with a twist and I have naturally been searching for a similarly named function. Thanks

@CrossEye
Copy link
Member

🌿

@davidchambers
Copy link
Member Author

It seems we're comfortable deprecating R.containsWith. The question is whether R.containsBy is actually a useful addition. Can anyone think of a situation in which it's more convenient than R.any?

@TheLudd
Copy link
Contributor

TheLudd commented Aug 25, 2015

If we only had R.any then all usage of containsBy would need to be changed from:

R.containsBy(someFn, someValue)

to

R.any(R.compose(R.eq(someValue), someFn))

Idk. I think these two cases look very weird and would certainly not miss it. I think this (and other functions) should only be added from a real need users of ramda feel that they have.

@davidchambers
Copy link
Member Author

I think this […] should only be added from a real need users of ramda feel that they have.

I agree. I suggest we open a separate pull request simply to deprecate R.containsWith. We can then close this pull request. If in the future a need for R.containsBy arises, we'll know where to look for an implementation. :)

This was referenced Sep 8, 2015
@davidchambers
Copy link
Member Author

We'll revisit this if there is demand for R.containsBy.

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.

None yet

3 participants