Skip to content
This repository

Seed RouteMatch params as long as params is set. This permits setting an empty array. #2285

Merged
merged 1 commit into from almost 2 years ago

5 participants

Bilge Don't Add Me To Your Organization a.k.a The Travis Bot Matthew Weier O'Phinney Dolf Schimmel Ben Scholzen
Bilge

No description provided.

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request passes (merged d465123 into 3a4cee6).

Dolf Schimmel Freeaqingme commented on the diff
library/Zend/Mvc/Controller/Plugin/Forward.php
@@ -136,7 +136,7 @@ public function dispatch($name, array $params = null)
136 136 }
137 137
138 138 // Allow passing parameters to seed the RouteMatch with
139   - if ($params) {
  139 + if (isset($params)) {
5
Dolf Schimmel Collaborator

Since params is always set, perhaps it would make more sense to simply check if it's not null? But then again, that is already what this if-clause was doing already, so I am kinda missing the significance of this PR. Perhaps you can elaborate?

Bilge
Bilge added a note

It is not the same. If you pass an empty array the old check will ignore it; isset() treats null and an empty array differently.

Ben Scholzen Collaborator

I'd argue with @Freeaqingme here, that it should likely be ($params !== null).

Bilge
Bilge added a note

What exactly do you think isset() does?

Ben Scholzen Collaborator

isset() primarily checks if a variable actually exists. Only secondarily it checks if it is not null. Using an implicit isset() check yields the option that $params could not have been set in the method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Matthew Weier O'Phinney weierophinney referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#2285] Wrote tests
- Provided tests to ensure you can pass an empty array for route matches
  when using the forward() plugin.
- Changed test from isset() to '!== null'
6b3602f
Matthew Weier O'Phinney weierophinney merged commit d465123 into from
Matthew Weier O'Phinney

Agreed with @DASPRiD and @Freeaqingme; using "isset()" will test for a non null, non-zero, non empty string, non empty array value. Since the typehint is for array, the only possible values are null or an array -- so testing for not null is more explicit and semantically correct.

I added tests to verify the change, and merged to release and master branches.

Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#2285] Wrote tests
- Provided tests to ensure you can pass an empty array for route matches
  when using the forward() plugin.
- Changed test from isset() to '!== null'
04db3cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Sep 01, 2012
Bilge Bilge Seed RouteMatch params as long as params is set. This permits setting…
… an empty array.
d465123
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 1 addition and 1 deletion. Show diff stats Hide diff stats

  1. +1 1  library/Zend/Mvc/Controller/Plugin/Forward.php
2  library/Zend/Mvc/Controller/Plugin/Forward.php
@@ -136,7 +136,7 @@ public function dispatch($name, array $params = null)
136 136 }
137 137
138 138 // Allow passing parameters to seed the RouteMatch with
139   - if ($params) {
  139 + if (isset($params)) {
140 140 $event->setRouteMatch(new RouteMatch($params));
141 141 }
142 142

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.