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

[9x] Remove redundant check in FormRequest::validated() #41115

Merged
merged 1 commit into from
Feb 21, 2022
Merged

[9x] Remove redundant check in FormRequest::validated() #41115

merged 1 commit into from
Feb 21, 2022

Conversation

fabio-ivona
Copy link
Contributor

@fabio-ivona fabio-ivona commented Feb 19, 2022

Hi!

this PR aims to remove a redundant check in Illuminate\Foundation\Http\FormRequest:

// Illuminate\Foundation\Http\FormRequest lines 213-222

public function validated($key = null, $default = null)
{
    if (! is_null($key)) {
        return data_get(
            $this->validator->validated(), $key, $default
        );
    }

    return $this->validator->validated();
}

the !is_null is used to return the entire validated set if a $key is not provided

but the same check is done inside the data_get() helper:

// Illuminate\Collections\helpers.php lines 46-49

function data_get($target, $key, $default = null)
{
    if (is_null($key)) {
        return $target;
    }
    //...
}

so it can be safely removed and the code can be made simpler

note the same optimization can be done in master, will open a new PR for it if this gets approved

@driesvints
Copy link
Member

@fabio-ivona 9.x is merged into master so no need for a separate PR 👍

@fabio-ivona
Copy link
Contributor Author

@driesvints my fault, thinking that master was already set up for v10 👼

@driesvints
Copy link
Member

@fabio-ivona it is indeed set up for v10.

@fabio-ivona
Copy link
Contributor Author

ok now I got it 👍

@taylorotwell taylorotwell merged commit a1671d4 into laravel:9.x Feb 21, 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.

3 participants