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

[8.x] Add when helper function #39373

Closed
wants to merge 1 commit into from
Closed

Conversation

4cc355-d3n13d
Copy link

@4cc355-d3n13d 4cc355-d3n13d commented Oct 26, 2021

This PR adds a new helper function when($condition, callable $callback, $otherwise = null).
This was inspired by Builder Query method when and I've found this is useful alongside with tap, transform & with helpers.

Some usage examples:

when(request('status_ids'), fn($ids) => $query->whereIn('status_id', explode(',', $ids)));
$filter->equal('user_id', __('fields.author_name'))->select(function ($id) {
    return when(User::find($id), fn(User $user) => [$user->id => "{$user->id} - {$user->name} - {$user->email}"]);
})->ajax('api/author');

Sure, no breaking changes here.

@GrahamCampbell GrahamCampbell changed the title Add when helper function [8.x] Add when helper function Oct 26, 2021
@4cc355-d3n13d
Copy link
Author

@GrahamCampbell and 50 lines above...

if (! function_exists('transform')) {
    /**
     * Transform the given value if it is present.
     *
     * @param  mixed  $value
     * @param  callable  $callback
     * @param  mixed  $default
     * @return mixed|null
     */

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

@derekmd
Copy link
Contributor

derekmd commented Oct 26, 2021

For both of the given examples, the transform() helper can be swapped in since the callback is only desired when a value is present/filled.

Current alternatives for the first example:

transform(
    request('status_ids'),
    fn ($ids) => $query->whereIn('status_id', explode(',', $ids))
);
if (request()->filled('status_ids')) {
    $query->whereIn('status_id', explode(',', request('status_ids')));
}
$query->when(
    request('status_ids'),
    fn ($query, $ids) => $query->whereIn('status_id', explode(',', $ids))
);
collect(
    explode(',', request('status_ids'))
)->filter()->whenNotEmpty(
    fn ($ids) => $query->whereIn('status_id', $ids)
);

The framework has generally stopped adding global helper functions (except for #39103?) so applying Illuminate\Support\Traits\Conditionable to a class for a new when() method is probably the preferred solution at this point.

I wrote the original transform() helper and barely use it because the resulting PHP code usually has single-line bloat. e.g., for your second example I'd put that string formatting in a UserPresenter decorator class method.

$filter
    ->equal('user_id', __('fields.author_name'))
    ->select(function ($id) {
        // this single line is still too complex for my liking
        return transform(User::find($id), fn ($user) => [$user->id => $user->present()->accountDetails]);
    })
    ->ajax('api/author');

Adding when() to that API would make conditional chaining more apparent and it reads like a natural English sentence:

$filter
    ->equal('user_id', __('fields.author_name'))
    ->when(User::find($id), function ($filter, $user) {
        return $filter->select(fn ($id) => [$user->id => $user->present()->accountDetails]);
    })
    ->ajax('api/author');

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.

4 participants