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.5] Fix misleading behavior in Route::getAction() #21083

Merged
merged 3 commits into from
Sep 8, 2017
Merged

[5.5] Fix misleading behavior in Route::getAction() #21083

merged 3 commits into from
Sep 8, 2017

Conversation

kamui545
Copy link
Contributor

@kamui545 kamui545 commented Sep 8, 2017

Patch for the following PR #20975

Pretty cool. However, am I the only one bothered by the fact that it is returning the action array if the key is not found?

Wouldn't it be more appropriate to return null instead as we expect a very specific property?

I find this misleading, especially if using it in a condition.
Eg: if ($route->getAction('controller')), even if there is no controllers it will return true...

As a bonus I added some tests.

@kamui545 kamui545 changed the title Fix misleading behavior in Route::getAction() [5.5] Fix misleading behavior in Route::getAction() Sep 8, 2017
@miclf
Copy link
Contributor

miclf commented Sep 8, 2017

For those who wonder, this PR covers all three cases with just a single line of code:

  1. Without any key, it still returns the entire array (thus preserving the old behaviour).
  2. If we give a key and if it exists, it returns the associated value.
  3. If we give a key and if it does not exist, it returns null.

@taylorotwell taylorotwell merged commit 2dc3da1 into laravel:5.5 Sep 8, 2017
@kamui545 kamui545 deleted the fix_get_action branch September 9, 2017 06:56
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