Skip to content

Commit

Permalink
fix [] -> array()
Browse files Browse the repository at this point in the history 
change separator "::" -> ":"
  • Loading branch information
Renat Bilalov committed Nov 25, 2013
1 parent ef56f5b commit fc816fb
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 18 deletions.
34 changes: 17 additions & 17 deletions Slim/Route.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ class Route

/**
* Constructor
* @param string $pattern The URL pattern (e.g. "/books/:id")
* @param mixed $callable Anything that returns TRUE for is_callable()
* @param string $pattern The URL pattern (e.g. "/books/:id")
* @param mixed $callable Anything that returns TRUE for is_callable()
*/
public function __construct($pattern, $callable)
{
Expand Down Expand Up @@ -154,8 +154,8 @@ public function getCallable()
*/
public function setCallable($callable)
{
$matches = [];
if(is_string($callable) && preg_match('!^([^\:]+)\:\:(.+)$!', $callable, $matches)) {
$matches = array();
if (is_string($callable) && preg_match('!^([^\:]+)\:([[:alnum:]]+)$!', $callable, $matches)) {
$callable = array(new $matches[1], $matches[2]);

This comment has been minimized.

Sign in to view

Copy link
@lalop

lalop Dec 24, 2013

Contributor

For performance reason, the controller should be created only if nessary, isn't it ?
At least when we are not on debug.
}

Expand Down Expand Up @@ -199,7 +199,7 @@ public function getName()
*/
public function setName($name)
{
$this->name = (string) $name;
$this->name = (string)$name;
}

/**
Expand All @@ -222,7 +222,7 @@ public function setParams($params)

/**
* Get route parameter value
* @param string $index Name of URL parameter
* @param string $index Name of URL parameter
* @return string
* @throws \InvalidArgumentException If route parameter does not exist at index
*/
Expand All @@ -237,8 +237,8 @@ public function getParam($index)

/**
* Set route parameter value
* @param string $index Name of URL parameter
* @param mixed $value The new parameter value
* @param string $index Name of URL parameter
* @param mixed $value The new parameter value
* @throws \InvalidArgumentException If route parameter does not exist at index
*/
public function setParam($index, $value)
Expand Down Expand Up @@ -356,7 +356,7 @@ public function matches($resourceUri)
$patternAsRegex = preg_replace_callback(
'#:([\w]+)\+?#',
array($this, 'matchesCallback'),
str_replace(')', ')?', (string) $this->pattern)
str_replace(')', ')?', (string)$this->pattern)
);
if (substr($this->pattern, -1) === '/') {
$patternAsRegex .= '?';
Expand All @@ -368,7 +368,7 @@ public function matches($resourceUri)
}
foreach ($this->paramNames as $name) {
if (isset($paramValues[$name])) {
if (isset($this->paramNamesPath[ $name ])) {
if (isset($this->paramNamesPath[$name])) {
$this->params[$name] = explode('/', urldecode($paramValues[$name]));
} else {
$this->params[$name] = urldecode($paramValues[$name]);
Expand All @@ -381,17 +381,17 @@ public function matches($resourceUri)

/**
* Convert a URL parameter (e.g. ":id", ":id+") into a regular expression
* @param array $m URL parameters
* @param array $m URL parameters
* @return string Regular expression for URL parameter
*/
protected function matchesCallback($m)
{
$this->paramNames[] = $m[1];
if (isset($this->conditions[ $m[1] ])) {
return '(?P<' . $m[1] . '>' . $this->conditions[ $m[1] ] . ')';
if (isset($this->conditions[$m[1]])) {
return '(?P<' . $m[1] . '>' . $this->conditions[$m[1]] . ')';
}
if (substr($m[0], -1) === '+') {
$this->paramNamesPath[ $m[1] ] = 1;
$this->paramNamesPath[$m[1]] = 1;

return '(?P<' . $m[1] . '>.+)';
}
Expand All @@ -401,7 +401,7 @@ protected function matchesCallback($m)

/**
* Set route name
* @param string $name The name of the route
* @param string $name The name of the route
* @return \Slim\Route
*/
public function name($name)
Expand All @@ -413,7 +413,7 @@ public function name($name)

/**
* Merge route conditions
* @param array $conditions Key-value array of URL parameter conditions
* @param array $conditions Key-value array of URL parameter conditions
* @return \Slim\Route
*/
public function conditions(array $conditions)
Expand All @@ -439,6 +439,6 @@ public function dispatch()
}

$return = call_user_func_array($this->getCallable(), array_values($this->getParams()));
return ($return === false)? false : true;
return ($return === false) ? false : true;
}
}
10 changes: 9 additions & 1 deletion tests/RouteTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,21 @@ public function testGetCallable()

public function testGetCallableAsClass()
{
$route = new \Slim\Route('/foo', '\Slim\Router::getCurrentRoute');
$route = new \Slim\Route('/foo', '\Slim\Router:getCurrentRoute');

This comment has been minimized.

Sign in to view

Copy link
@lalop

lalop Dec 23, 2013

Contributor

where does this notation come from ?

This comment has been minimized.

Sign in to view

Copy link
@designermonkey

designermonkey Dec 23, 2013

Member

It was discussed by a few and decided with @codeguy's input that normal class methods would use : and static methods use ::

This comment has been minimized.

Sign in to view

Copy link
@lalop

lalop Dec 23, 2013

Contributor

ok but in fact I have never seen this notation before, and I can't find it in the php documentation.
My question is more on the php side than on the Slim one.

This comment has been minimized.

Sign in to view

Copy link
@lalop

lalop Dec 23, 2013

Contributor

Or maybe that is internal of Slim ???

This comment has been minimized.

Sign in to view

Copy link
@marcioAlmada

marcioAlmada Dec 23, 2013

Contributor

@lalop Definitely not part of PHP syntax.

This comment has been minimized.

Sign in to view

Copy link
@codeguy

codeguy Dec 23, 2013

Member

We needed a syntax for this specific purpose that did not collide with an existing syntax used for other purposes. For this PR, the "::" separator was originally suggested, but that is already used to invoke static methods. So we settled on ":"... other frameworks use "@", ":", etc.

This comment has been minimized.

Sign in to view

Copy link
@lalop

lalop Dec 24, 2013

Contributor

Ok, I understand.
Thank you

This comment has been minimized.

Sign in to view

Copy link
@lalop

lalop Jan 14, 2014

Contributor

Sorry to come back here but I'm trying to use this syntax and I meet some trouble with it.
If the controller is instanciated like that how can it access to the application object to render a view for exemple ?
There is no kind of magic method to set the application in the object ? Or maybe it can be past like a last arguments of the route's method.
Am I missing something ?

$callable = $route->getCallable();
$this->assertInstanceOf('\Slim\Router', $callable[0]);
$this->assertEquals('getCurrentRoute', $callable[1]);
}

public function testGetCallableAsStaticMethod()
{
$route = new \Slim\Route('/bar', '\Slim\Slim::getInstance');

$callable = $route->getCallable();
$this->assertEquals('\Slim\Slim::getInstance', $callable);
}

public function testSetCallable()
{
$callable = function () {
Expand Down

0 comments on commit fc816fb

Please sign in to comment.