MVC controller / PHP request dispatcher

Question

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<br>";
        }

        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.<br>";
        }
    }

    function prepareController($__REQUEST__) {
        $this->controller = ucfirst($__REQUEST__->getController()) . "Controller";
        $this->controller = '\MVC\controllers\\' . $this->controller;
    }
}

new Index();

routing.php (prepended file to any request)

<?php
    if (preg_match('/\.(?:png|jpg|jpeg|gif)$/', $_SERVER["REQUEST_URI"])) {
        return false;
    } else {
        include 'index.php';
    }

URI_Request.php

<?php

namespace MVC\libs;

class URI_Request {

    private $request;
    private $pairs;
    private $controller;
    private $method;

    function __construct($request) {
        $this->request = trim($request);
        $this->request = explode("/", $this->request);
        $this->request = array_filter($this->request);
        $this->request = $this->handle($request);
    }

    function handle($request) {

        foreach($this->request as $key=>$item) {
            switch($key) {
                case 1:
                    $this->controller = $item;
                break;
                case 2:
                    $this->method = $item;
                break;
                default:
                    if($key > 2) {
                        $item = explode("=", $item);
                        if(isset($item[1])) {
                            //key item pair, data
                            $this->pairs[$item[0]] = $item[1];
                        } else if(isset($item[0])) {
                            echo "<b>WARNING: \"" . $item[0] . "\" has no key/value!</b>";
                        }
                    }
                break;
            }
        }

        if(!$this->controller){
            $this->controller = 'index';
        }

        if(!$this->method) {
            $this->method = 'index';
        }

        if(!$this->pairs) {
            $this->pairs = [];
        }
    }

    function getController() {
        return $this->controller;
    }

    function getMethod() {
        return $this->method;
    }

    function getPairs() {
        return $this->pairs;
    }

    function getRequest() {
        return $this->request;
    }
}

Autoloader.php

<?php

namespace MVC;

class Autoloader {

    function __construct() {
        session_start();
        spl_autoload_register('self::loader');
    }

    static public function loader($className) {
        $filename = str_replace('\\', '/', $className) . '.php';
        $filename = str_replace('MVC/', '', $filename);
        if(file_exists($filename)) {
            require($filename);
        }
    }
}

Answer 1

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 < $argumentCount; $i++) {
        $item = explode("=", $this->request[$i]);
        if(isset($item[1])) {
            $this->pairs[$item[0]] = $item[1];
        } else if(isset($item[0])) {
            echo "<b>WARNING: \"" . htmlspecialchars($item[0], ENT_QUOTES, 'UTF-8') . "\" has no key/value!</b>";
        }
    }
}

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.

Answer 2

This was originally a comment, but it became too long, so I made it into an answer. I'm not sure this does definately answer your question, nor does it really review your code.

The main issue in your question is:

"I've noticed a small increment in time between requesting controllers/methods"

but you don't say what you compare it with. Can you tell us?

Also, how big an increment are we talking about? Also specify platform and PHP version. As for your question, apart from a few somewhat awkward OOP choices, I don't see anything that could really slow things down substantially.

My guess is that the delay is somehow in the server. It is given the URL site.com/login/recover and somehow you end up loading site.com/index.php. You don't tell us how, except that you don't use .htaccess, but my guess is that that's the issue.