HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Router to match URL to Controller Method

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
methodroutermatchcontrollerurl

Problem

I wrote a router class which accepts a URL and calls the appropriate controller method based on it.

I'm a little worried about the amount of dependencies it has (eg ErrorController, RequestMethod, etc), because it might make reusing it in other projects difficult.

It feels especially bad because a lot of the dependencies are static (and there might be even more in the future; eg if I add support for multiple languages). On the other hand, I also did not want to pass too many arguments to the router, and I'm not sure if it makes sense to create new objects every time for each of the other classes, and I also did not want them cluttering up my private fields (although that might be best?).

I don't write all that much in PHP, so I'm happy about any other comments regarding code quality as well.

```
connection = $connection;
try {
$serverRequestMethod = RequestMethod::parseRequestMethod();
} catch(UnknownRequest $unknown) {
ErrorController::notFound("illegal request method", $unknown);
}
$url = $this->cleanURL($requestUrl);
$this->route($url, $serverRequestMethod, $this->routeIdToRegex($routes));
}

/**
* calls a controller method based on the request url and defined routes.
*
* @param string $url the requested url, eg /user/5
* @param string $requestType POST, GET, PUT, or DELETE
* @param array $routes a whitelist of allowed urls + request types mapped to a controller + method. Example entry: "GET /" => "UserController.listAll",
* @return type|null return-value of called controller method. If url is not defined in the routes, ErrorController::notFound will be called.
*/
private function route($url, $requestType, $routes) {
$url = $this->removeQuery($url);
foreach ($routes as $route => $controllerAndMethod) {
if (String::startsWith($route, $requestType) && preg_match("~^" . $this->c

Solution

Working with methods is easier than configuration arrays. Configuring routes through method calls gives some overhead. If you do not feel it then there is nothing to worry about. If you do however, you could even create a code configurator that is called only once, to obtain configuration array, and result is cached. If decent IDE is in play, then adding route with fluent style configurator would be a breeze. You do not need to know configuration array structure and it can even change at background. That structure is used by another object some where else and it is part of that implementation detail.

Decouple your concerns by implementing strategies. Instead of passing all controller dependencies via router use dependency injection. Using those two principles alone gives you more flexibility.

You can create specific contract and implementation for it that calls static method for you if you must have them.

I hope this rambling puts you on some track. Please, feel free to ask questions! (:

p.s. hope that this is not considered as tumbleweed

Context

StackExchange Code Review Q#114912, answer score: 3

Revisions (0)

No revisions yet.