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

Merged
merged 1 commit into from
Mar 25, 2018

Conversation

JosephSilber
Copy link
Member

@JosephSilber 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 Add callback support to "optional" [5.6] Add callback support to "optional" Mar 25, 2018
@BrandonShar
Copy link
Contributor

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
Copy link
Member Author

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

@BrandonShar
Copy link
Contributor

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
@JosephSilber JosephSilber deleted the optional-callback branch March 25, 2018 17:35
@derekmd
Copy link
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
Copy link
Member

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
Copy link
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
Copy link
Member Author

@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants