-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Added else callable for Collection::when and Collection::unless #19140
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
base: add-collection-convenience-methods
Are you sure you want to change the base?
Added else callable for Collection::when and Collection::unless #19140
Conversation
| * @inheritDoc | ||
| */ | ||
| public function unless(mixed $condition, callable $callback): CollectionInterface | ||
| public function unless(mixed $condition, callable $callback, ?callable $else = null): CollectionInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So unless() is now just when() with 2nd and 3rd arguments swapped. We can avoid increasing the API surface by only keeping when() will 2nd and 3rd arguments both nullable (we can throw an exception if at least one of them is not passed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if the user wants a unless behavior they should use this?
$collection->when($user->is_admin, else: fn($it) => $it->filter(....))
I'm not sure if this is clear
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do unless() and when() need support for else? The two methods already give you a way to do if/else style filtering logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when is normally a conditional 'flow' with two parts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If your condition is the result of a function you don't need to assign the value to variable,
//One method call
$collection->when(
$Table->exists(['id' => 10]),
fn($it) => $it->filter(....),
fn($it) => $it->filter(....)
);
//instead of multiple calls
$collection->when(
$Table->exists(['id' => 10]),
fn($it) => $it->filter(....),
)->unless(
$Table->exists(['id' => 10]),
fn($it) => $it->filter(....),
);
//instead of using a var
$condition = $Table->exists(['id' => 10]);
$collection->when(
$condition,
fn($it) => $it->filter(....),
)->unless(
$condition,
fn($it) => $it->filter(....),
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair I would prefer to negate the condition and have
$collection->when(!$user->is_admin, fn($it) => $it->filter(....)) than $collection->when($user->is_admin, else: fn($it) => $it->filter(....))
Added else callable for Collection::when and Collection::unless
Now we have the else part of a condition