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

[9.x] allow Collection random() to accept a callable #43028

Merged
merged 1 commit into from
Jul 1, 2022

Conversation

browner12
Copy link
Contributor

this allows the user to, rather than passing an int directly, pass a closure that can reference back to the original Collection in case they need to reference the size of the Collection

a little confusing, so here's an example:

We have a User and Comments. Users can have many Comments. Let's say we want to show 5 random comments from a User on a dashboard.

$comments = $user->comments->random(5);

This probably seems fine, but we run into an issue if the User doesn't have at least 5 comments, and Laravel will throw an InvalidArgumentException. So we want to amend this to say we want 5 comments or the total count of the comments, whichever is smaller. In order to do this currently, you need to use a temporary variable.

$comments = $user->comments;
$comments = $comments->random(min(5, count($comments)));

With this PR you'll now be able to chain this entire call:

$comments = $user->comments->random(fn($items) => min(5, count($items)));

I'm still a little new to the generics docblocks, so LMK if that needs an adjustment.

this allows the user to, rather than passing an int directly, pass a closure that can reference back to the original Collection in case they need to reference the size of the Collection
@taylorotwell taylorotwell merged commit 3b7e82d into laravel:9.x Jul 1, 2022
@browner12 browner12 deleted the collection-random branch July 2, 2022 01:15
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