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

[5.7] Add "some" method to Collection as an alias to "contains" #26376

Merged
merged 2 commits into from
Nov 5, 2018
Merged

[5.7] Add "some" method to Collection as an alias to "contains" #26376

merged 2 commits into from
Nov 5, 2018

Conversation

paras-malhotra
Copy link
Contributor

After going through the Collections documentation, I couldn't find an any method, especially after observing that an every method does exist!

After quite a long code walkthrough, I found that contains would the equivalent of any. However, it's not quite apparent especially from a predicate callback perspective. I think adding any as an alias to contains would be very helpful. Since this is an additional method, there are no breaking changes.

@devcircus
Copy link
Contributor

Based on other methods in the framework, I would expect an 'any' method to accept multiple parameters and return whether or not the collection contains "any" of those. It doesn't feel like any == contains.

@paras-malhotra
Copy link
Contributor Author

paras-malhotra commented Nov 3, 2018

IMO, a method that returns whether or not a collection contains any params, sounds like a where or a whereAny.

I'll demonstrate why I think any is a perfect alias for contains. Mentioned below are some use cases:

  1. Check if any order has a book product:
$orders->any('product', 'book');
  1. Check if any order has more than 5 items:
$orders->any('quantity', '>', 5);
  1. Check if any order was paid by cash (truth test):
$orders->any(function($order) {
    return $order->paymentMethod() === 'cash';
});

All of the above use cases look perfect for an any method. What's more is that they're similar to every that has all the above use cases in a different context.

That's why I think any should be an alias for contains.

@phanan
Copy link
Contributor

phanan commented Nov 3, 2018

@paras-malhotra I think the term you're looking for here is some instead of any, knowing that:

  • every will return true if all predicate is true
  • some will return true if any predicate is true

@paras-malhotra
Copy link
Contributor Author

@phanan I agree. I checked the documentation of both Lodash and Underscore JS. Both of them have every and some methods that do the same thing that we're trying to do! I think this naming would also get rid of the confusion that @devcircus mentioned.

I'll update my PR to rename the method to some.

@paras-malhotra paras-malhotra changed the title [5.7] Add "any" method to Collection as an alias to "contains" [5.7] Add "some" method to Collection as an alias to "contains" Nov 4, 2018
@taylorotwell taylorotwell merged commit 5e47cb6 into laravel:5.7 Nov 5, 2018
@paras-malhotra paras-malhotra deleted the add_any_to_collection branch November 5, 2018 14:08
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

4 participants