patternphpMinor
Rewriting dispatch method
Viewed 0 times
methoddispatchrewriting
Problem
Is it possible to rewrite this piece of code any better when it comes to efficiency and/or readability?
public function dispatch()
{
$key = array_search($this->reqUri, $this->routes);
if ($key !== false) {
if (isset($this->methods[$key])) {
if ($this->methods[$key] === $this->reqMethod) {
call_user_func($this->callables[$key]);
return;
} else {
$this->defaultCallable();
return;
}
}
call_user_func($this->callables[$key]);
return;
}
$this->defaultCallable();
}Solution
The
In this arrangement, the lookup should be in \$O(1)\$ time or close, even with large number of routes.
Do you really need the
Can't it be simply:
In the innermost
In terms of readability, I find it slightly better this way:
That is, the
array_search will iterate over every element in $this->routes. That's not scalable. It would be better to reorganize $this->routes so that you can lookup keys more directly, so that your code will work like this:if (isset($this->routes[$this->reqUri])) {
$key = $this->routes[$this->reqUri];
// ...
}In this arrangement, the lookup should be in \$O(1)\$ time or close, even with large number of routes.
Do you really need the
!== false here?if ($key !== false) {Can't it be simply:
if ($key) {In the innermost
if, since you return in both the if and the else, you could move the return statement outside:if (isset($this->methods[$key])) {
if ($this->methods[$key] === $this->reqMethod) {
call_user_func($this->callables[$key]);
} else {
$this->defaultCallable();
}
return;
}In terms of readability, I find it slightly better this way:
if ($key) {
if (isset($this->methods[$key])) {
if ($this->methods[$key] === $this->reqMethod) {
call_user_func($this->callables[$key]);
return;
}
} else {
call_user_func($this->callables[$key]);
return;
}
}
$this->defaultCallable();That is, the
$this->defaultCallable() call appears only once instead of twice, everything else still being the same.Code Snippets
if (isset($this->routes[$this->reqUri])) {
$key = $this->routes[$this->reqUri];
// ...
}if ($key !== false) {if ($key) {if (isset($this->methods[$key])) {
if ($this->methods[$key] === $this->reqMethod) {
call_user_func($this->callables[$key]);
} else {
$this->defaultCallable();
}
return;
}if ($key) {
if (isset($this->methods[$key])) {
if ($this->methods[$key] === $this->reqMethod) {
call_user_func($this->callables[$key]);
return;
}
} else {
call_user_func($this->callables[$key]);
return;
}
}
$this->defaultCallable();Context
StackExchange Code Review Q#56272, answer score: 4
Revisions (0)
No revisions yet.