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] add findOr method to Query/Builder #42290

Merged
merged 2 commits into from
May 9, 2022

Conversation

mihaileu
Copy link
Contributor

@mihaileu mihaileu commented May 6, 2022

This PR adds a findOr() method to the Query Builder. It is similar with #42092

Example usage:

$data = \DB::table('something')->findOr($id, fn () => throw new CustomException);
$data = \DB::table('something')->findOr($id, fn () => (object) ['id=>'0', 'name'=>'N/A']);

Edit: I updated the example as it was wrong.
Probably there are other usage cases.

@mihaileu mihaileu changed the title add findOr method to Query/Builder [9.x] add findOr method to Query/Builder May 6, 2022
@eduance
Copy link
Contributor

eduance commented May 8, 2022

Another way of doing this is by using the the ?: operator, but for readability, I'd really like this addition.

@bert-w
Copy link
Contributor

bert-w commented May 8, 2022

No support because you can do the same thing with plain PHP:

$id = DB::table('users')->find(9876)?->id ?? throw new Exception('nope');

vs

$id = DB:table('users')->findOr(9876, throw new Exception('nope'));

(also the $callback parameter cannot be null in your implementation)

@GrahamCampbell
Copy link
Member

I'm 👎 here, for the reason provided by @bert-w. Since PHP 8.0, throw statements are also expressions.

@davisngl
Copy link

davisngl commented May 8, 2022

And yet none of you had problems with #42092, when the usage with throwing an exception was highlighted?

@bert-w
Copy link
Contributor

bert-w commented May 8, 2022

And yet none of you had problems with #42092, when the usage with throwing an exception was highlighted?

good catch, didnt see that PR since I only browse here occasionally. However that being said I'd take the same stance. Any of the doSomethingOr functions I dont really like since they can often times be achieved with other methods like stated above. So the functions are bloat.

Anyway that means that this function is already merged so RIP

@davisngl
Copy link

davisngl commented May 8, 2022

And yet none of you had problems with #42092, when the usage with throwing an exception was highlighted?

good catch, didnt see that PR since I only browse here occasionally. However that being said I'd take the same stance. Any of the doSomethingOr functions I dont really like since they can often times be achieved with other methods like stated above. So the functions are bloat.

Anyway that means that this function is already merged so RIP

True.

Would say more, but this isn't the community for being able to take constructive criticism.

@bert-w
Copy link
Contributor

bert-w commented May 8, 2022

Apparently there's also #42253 , no clue why these all pop up around the same time. It is kinda weird especially since Laravel 9.x now requires PHP 8+ which adds nice functionality to the language itself. I think it will take a couple of years (yes) for people to start using features frequently like the null safe ($obj->user()?->id) operators...

@taylorotwell taylorotwell merged commit e02307f into laravel:9.x May 9, 2022
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