MVC controller / PHP request dispatcher

After getting the requested controller from the URI request, it checks whether this exists and whether the requested method (default index) exists within that controller, then calls them.

My intention is to have everything running object oriented so I'm trying to remove any procedural/unattached variables that may appear throughout the code.

My question is, can this code be refined to be more efficient? It works fine, however I've noticed a small increment in time between requesting controllers/methods and I'm not sure where this slow behaviour is coming from.

Note: the intention of this code is to check the URI string (I don't use .htaccess for this) and request a controller and a method from the URI string, for example: site.com/controller/method, site.com/login/recover

index.php

```
namespace MVC;
use MVC\libs\URI_Request;

class Index {

private $controller;
private $method;

function __construct() {

$this->autoload();

$__REQUEST__ = new URI_Request($_SERVER["REQUEST_URI"]);

$this->prepareController($__REQUEST__);
$this->method = $__REQUEST__->getMethod();

if($this->checkClass()) {
$this->controller = new $this->controller();

if($this->checkMethod($__REQUEST__)) {
$this->controller->{$this->method}();
}
}
}

function autoload() {
require_once("Autoloader.php");
new Autoloader();
}

function checkClass() {
if(class_exists($this->controller)) {
return true;
} else {
echo "controller: error (" . $this->controller . ") does not exist";
}

return false;
}

function checkMethod($request) {
if(method_exists($this->controller, $this->method)) {
return true;
} else {
echo "method: (" . $this->method . ") does not exist for requested controller.";
}
}

function prepareController($__REQUEST__) {

I can't really help with performance, but if you suspect the URI_Request handler, here are some ideas:

• get rid of array_filter and replace it with $this->request = rtrim($request, "/");, which should be faster, and is also easier to understand.

• remove $this->request = trim($request);, as it's not needed.

I would probably also rewrite the handle method (it might not necessarily be faster, but I think it's a lot easier to read):

function handle($request) {
    if (!isset($this->request[1])) {
        $this->controller = 'index';
        return;
    }

    if (!isset($this->request[2])) {
        $this->method = 'index';
        return;
    }

    $argumentCount = count($this->request);
    for ($i = 3; $i request[$i]);
        if(isset($item[1])) {
            $this->pairs[$item[0]] = $item[1];
        } else if(isset($item[0])) {
            echo "WARNING: \"" . htmlspecialchars($item[0], ENT_QUOTES, 'UTF-8') . "\" has no key/value!";
        }
    }
}

Misc

• magic numbers in URI_Request: I would consider 1 and 2 magic numbers in this case, and would pass them to the function as argument. If your application is ever moved into a sub-directory, a user needs to be able to control these variables.

• I wouldn't echo in the URI_Request class. If more than one pair of arguments is wrong, you will get a lot of warnings. Also, maybe there will be a point when you want to make a proper 4xx/5xx error page, and then it will be nice to have all those echos in one place already.

• I would get rid of the autoload function, because it doesn't really do anything. It's name suggest that it loads something automatically, but it only instantiates the autoloader class.

• checkClass and checkMethod should probably behave the same, but one returns false and one doesn't.

• also, if the failure message is echoed in the else case, I would move the return false there as well.

StackExchange Code Review Q#84460, answer score: 2