First MVC Router

I'm learning the mvc pattern and today I finished my first mvc routing class.
Hope you can give me some tipps and improvements.

Router.php

splitUrl();
        $this->setRoute();
    }

    /**
     * Split the url
     *
     * @return void
     */
    public function splitUrl()
    {
        if (isset($_GET['url']) && !empty($_GET['url'])) {
            $this->_request = trim(filter_var(strtolower($_GET['url']), FILTER_SANITIZE_URL));
            $this->_request = explode('/', rtrim($this->_request, '/'));
        } else {
            $this->_request = null;
        }
    }

    /**
     * Check the request and load the controller
     *
     * @return void
     */
    public function setRoute()
    {
        if (isset($this->_request[0])) {
            if (file_exists(ROOT . '/app/controllers/' . $this->_request[0] . '.php')) {
                $this->_controller = $this->_request[0];
            } else {
                $this->_controller = 'error';
            }
        }

        $this->_controller = 'App\Controllers\\' . ucfirst($this->_controller);
        $this->_controller = new $this->_controller;

        if (isset($this->_request[1])) {
            if (method_exists($this->_controller, $this->_request[1] . 'Action')) {
                $this->_action = $this->_request[1];
            } else {
                $this->_action = 'error';
            }
        }

        unset($this->_request[0], $this->_request[1]);

        if (isset($this->_request[2])) {
            $this->_param = array_values($this->_request);
        }

        call_user_func_array([$this->_controller, $this->_action . 'Action'], $this->_param);
    }

    /**
     * Destory the variables
     *
     * @return void
     */
    public function __destruct()
    {
        $this->_param      = null;
        $this->_model      = null;
        $this->_action     = null;
        $this->_request    = null;
        $this->_controller = null;
    }
}

?>

At first glance, this router is doing too much:

• Extracts relevant information from the HTTP request

• Instantiates a controller object

• Executes a method on the controller

• Does a little error handling if no controller exists

Really a router should only be responsible for item #1: Extracting relevant information from the HTTP request, and return that information in the form of a concrete object. What you have is the name of a controller, the name of an action, and an array of values to pass into the controller action:

class RouteData
{
    private $controllerName;
    private $controllerAction;
    private $arguments;

    public function __construct($controllerName, $controllerAction, $arguments) {
        $this->controllerName = $controllerName;
        $this->controllerAction = $controllerAction;
        $this->arguments = isset($arguments) ? $arguments : array();
    }

    public function getControllerAction() {
        return $this->controllerAction;
    }

    public function getControllerName() {
        return $this->controllerName;
    }

    public function getArguments() {
        return $this->arguments;
    }
}

You should provide all values in the constructor so the object is immutable. The route data should not change once it has been created.

Now that we've encapsulated the route data, the constructor of the Router class also does too much. It should be refactored so that it doesn't do anything but take in the data it needs to do its job. Offhand, the super global $_GET variable is used. This would be better if it were passed in to the method that creates the route data:

class Router
{
    public function getRouteData($getParams) {
        // Extract info, return a new RouteData object
    }
}

Your router doesn't really need persistent state. The splitUrl method should return an array of values instead of setting properties on the Router object. In fact, renaming this to createRouteValues and passing in the $_GET array would be better:

private function createRouteValues($getParams) {
    $routeValues = array();

    if (isset($getParams['url']) && !empty($getParams['url'])) {
        $url = trim(filter_var(strtolower($getParams['url']), FILTER_SANITIZE_URL));
        $routeValues = explode('/', rtrim($url, '/'));
    }

    return $routeValues;
}

Now it's just a matter of pulling out the necessary pieces of the array and creating the RouteData object:

public function getRouteData($getParams) {
    $routeValues = $this->createRouteValues($getParams);
    $controllerName = isset($routeValues[0]) ? $routeValues[0] : 'error';
    $controllerAction = isset($routeValues[1]) ? $routeValues[1] : 'index';
    $arguments = count($routeValues) > 2 ? array_slice($routeValues, 2) : array();

    return new RouteData($controllerName, $controllerAction, $arguments);
}

Notice that the Router does not do any error handling, nor does it try to resolve a controller name to a class, create the controller or execute the action. It's only responsible for turning an HTTP request into a RouteData object. Something else in the system needs to take the RouteData, get a controller object, execute an action on the controller and perform error handling. For that, we need two more classes. One that creates the controller object, and another that invokes an action on the controller and does some error handling.

First, the controller factory:

class ControllerFactory
{
    private $rootDirectory;

    public function __construct($rootDirectory) {
        $this->rootDirectory = $rootDirectory;
    }

    public function createController(RouteData route) {
        $filePath = $this->createFilePath(route->getControllerName());

        if (!file_exists($filePath)) {
            return new ErrorController();
        }

        $class = 'App\Controllers\\' . ucfirst($route->getControllerName());
        $controller = new $class();

        return $controller;
    }

    public function getErrorController() {
        return new ErrorController();
    }

    private function createFilePath($controllerName) {
        return $this->rootDirectory . '/app/controllers/' . $controllerName . '.php';
    }
}

The controller factory does a little error handling, in that it will return an ErrorController object if the route does not resolve to a controller class. It also provides a public getErrorController method that will be used to return a generic "error controller object" if one is needed.

The last thing we will do is create an "application" class that glues all the pieces together:

```
class Application
{
private $controllerFactory;
private $router;

public function __construct(ControllerFactory $controllerFactory, Router $router) {
$this->controllerFactory = $controllerFactory;
$this->router = $router;
}

public function handleRequest() {
try {
$route = $this->router->getRouteData($_GET);
$co

StackExchange Code Review Q#113475, answer score: 3