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.6] Add callback support to "optional" #23688

Merged
merged 1 commit into from Mar 25, 2018

Conversation

Projects
None yet
6 participants
@JosephSilber
Contributor

JosephSilber commented Mar 25, 2018

Lets you pipe the value into an outside function, with the guarantee that you're not passing it null:

return optional($modelOrNull, function ($definitelyTheModel) {
    return doSomethingWithTheModel($definitelyTheModel);
});

@JosephSilber JosephSilber changed the title from Add callback support to "optional" to [5.6] Add callback support to "optional" Mar 25, 2018

@BrandonShar

This comment has been minimized.

Contributor

BrandonShar commented Mar 25, 2018

how is this better than a ternary or an if statement?

I love the optional helper, but I think it's getting stretched a bit far lately.

@JosephSilber

This comment has been minimized.

Contributor

JosephSilber commented Mar 25, 2018

A ternary is insufficient when the value comes from a function/expression.

@BrandonShar

This comment has been minimized.

Contributor

BrandonShar commented Mar 25, 2018

Ah ok, so this would be for cases where you might have a result of one function that you want to pass as an argument to another function. I don't think that's a common pattern for me, but that may be because I haven't been looking for it. It would probably be helpful to update the example since the current one would be more succinct as a ternary.

There was another PR recently where a contributor wanted to add similar functionality to tap and I thought creating a when helper might be a better idea. Maybe that would be the same case here?

Personally, I like keeping optional as nothing more than a PHP version of Ruby's & operator, so I have a bit of a bias.

@taylorotwell taylorotwell merged commit cb6e308 into laravel:5.6 Mar 25, 2018

2 checks passed

continuous-integration/styleci/pr The StyleCI analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@JosephSilber JosephSilber deleted the JosephSilber:optional-callback branch Mar 25, 2018

@derekmd

This comment has been minimized.

Contributor

derekmd commented Mar 27, 2018

transform() used internally by API resources can do this: https://laravel.com/docs/5.6/helpers#method-transform.

It will call the Closure when the value passes filled().

https://github.com/laravel/framework/blob/5.6/src/Illuminate/Support/helpers.php#L1121-L1142

Otherwise it returns null by default.

@crynobone

This comment has been minimized.

Contributor

crynobone commented Mar 27, 2018

I find it odd that optional() may now not return Illuminate\Support\Optional.

optional($user, function ($user) {
    return $user->sendResetPassword();
})->getInvalidProperty->dump;

It now would throws exception.

@garygreen

This comment has been minimized.

Contributor

garygreen commented Mar 27, 2018

@BrandonShar

I love the optional helper, but I think it's getting stretched a bit far lately.

Do a PR to remove the Optional helper. I'd +1 it. tap tap tap tap dat all day 😄

@JosephSilber

This comment has been minimized.

Contributor

JosephSilber commented Mar 27, 2018

@derekmd the transform helper checks for filled, as you've noted. Not null.

@crynobone I don't see that confusing at all. These are two different uses of the optional helper. Think of it as an overloaded method with a different return type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment