PHP Routing with MVC Structure

Question

I'm trying to do a simple CMS with PHP from scratch using MVC structure.

Yesterday I posted this, which is a login system using PHP and it works but it has a handful of problems regarding the OOP aspects of it.

This is kind of the version 2. It doesn't have the login functionality, it's just a "routing system" that will support different request methods and supports dynamic routing (URL parameters and query strings).

This is the project structure:

.
+-- classes
|   +-- Database.php
|   +-- Route.php
+-- controllers
|   +-- AboutUs.php
|   +-- Controller.php
+-- views
|   +-- about-us.php
+-- .htaccess
+-- index.php
+-- routes.php

I'm gonna write here what I have in each file:

Database.php

<?php

class Database{

    public static $host = 'localhost';
    public static $dbName = 'cms';
    public static $username = 'root';
    public static $password = '';

    private static function connect(){
        $pdo = new PDO('mysql:host='.self::$host.';dbname='.self::$dbName.';charset=utf8', self::$username, self::$password);
        $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        return $pdo;
    }

    public static function query($query, $params = array()){
        $statement = self::connect()->prepare($query);
        $statement->execute($params);
        if(explode(' ', $query)[0] == 'SELECT'){
            $data = $statement->fetchAll();
            return $data;
        }
    }
}

?>

Route.php - this is where I made the most work

<?php

class Route {
    public static function get($route, $function){
        //get method, don't continue if method is not the 
        $method = $_SERVER['REQUEST_METHOD'];
        if($method !== 'GET'){ return; }

        //check the structure of the url
        $struct = self::checkStructure($route, $_SERVER['REQUEST_URI']);

        //if the requested url matches the one from the route
        //get the url params and run the callback passing the params
        if($struct){
            $params = self::getParams($route, $_SERVER['REQUEST_URI']);
            $function->__invoke($params);

            //prevent checking all other routes
            die();
        }
    }

    public static function urlToArray($url1, $url2){
        //convert route and requested url to an array
        //remove empty values caused by slashes
        //and refresh the indexes of the array
        $a = array_values(array_filter(explode('/', $url1), function($val){ return $val !== ''; }));
        $b = array_values(array_filter(explode('/', $url2), function($val){ return $val !== ''; }));

        //debug mode for development
        if(true) array_shift($b);
        return array($a, $b);
    }

    public static function checkStructure($url1, $url2){
        list($a, $b) = self::urlToArray($url1, $url2);

        //if the sizes of the arrays don't match, their structures don't match either
        if(sizeof($a) !== sizeof($b)){
            return false;
        }

        //for each value from the route
        foreach ($a as $key => $value){

            //if the static values from the url don't match
            // or the dynamic values start with a '?' character
            //their structures don't match
            if($value[0] !== ':' && $value !== $b[$key] || $value[0] === ':' && $b[$key][0] === '?'){
                return false;
            }
        }

        //else, their structures match
        return true;
    }

    public static function getParams($url1, $url2){
        list($a, $b) = self::urlToArray($url1, $url2);

        $params = array('params' => array(), 'query' => array());

        //foreach value from the route
        foreach($a as $key => $value){
            //if it's a dynamic value
            if($value[0] == ':'){
                //get the value from the requested url and throw away the query string (if any)
                $param = explode('?', $b[$key])[0];
                $params['params'][substr($value, 1)] = $param;
            }
        }

        //get the last item from the request url and parse the query string from it (if any)
        $queryString = explode('?', end($b))[1];
        parse_str($queryString, $params['query']);

        return $params;
    }
}

?>

AboutUs.php - This is just a blank class extending the controller, for testing purposes.

<?php

class AboutUs extends Controller{
    
}

?>

Controller.php

<?php

class Controller extends Database{

    public static function CreateView($viewName, $urlParams = null){
        require_once('./views/'.$viewName.'.php');
    }
}

?>

about-us.php - This is the view, has nothing but a heading, just for testing

<h1>About Us</h1>

.htaccess

RewriteEngine On

RewriteRule ^([^/]+)/? index.php?url=$1 [L,QSA]

index.php - Autoloading all the classes and requiring the routes file

<?php

    function loadClasses($class_name){
        $classesPath = './classes/'.$class_name.'.php';
        $controllersPath = './controllers/'.$class_name.'.php';

        if(file_exists($classesPath)){
            require_once($classesPath);
        }else if(file_exists($controllersPath)){
            require_once($controllersPath);
        }    
    }

    spl_autoload_register(loadClasses);

    require_once('routes.php');
?>

routes.php - All the routes will be here

<?php

Route::get('/about-us', function(){
    AboutUs::CreateView('about-us');
});

?>

---

Basically the routing system works like this.

1. First, it checks the request method, if it matches, it continues to test the structure of the URLs.

   The structure of the route I set on the routes.php should match the structure of the route the user is accessing.

2. If this is the case, it parses the requested URL and looks for URL parameters and query strings, using the route URL as a base.

3. Then it invokes the callback function passing the URL parameters with an associative array.

4. Then I use a die() statement to stop executing the code. I made this because when I have multiple routes, it will check for the structure of everyone even if one just matched.

BTW, I have only the GET method right now but then when I add more, I'll probably put this into a function and use that in every request method:

//check the structure of the url
$struct = self::checkStructure($route, $_SERVER['REQUEST_URI']);

//if the requested url matches the one from the route
//get the url params and run the callback passing the params
if($struct){
    $params = self::getParams($route, $_SERVER['REQUEST_URI']);
    $function->__invoke($params);

    //prevent checking all other routes
    die();
}

Examples from the routing:

Routes:

/users/:username
/users/:username/photos
/users/:username/photos/:photoId
/users/:username/photos/:photoId?showComments=false

Requested URLS:

/users/jake1990
/users/jake1990/photos
/users/jake1990/photos/931280134
/users/jake1990/photos/931280134?showComments=false

Callback $urlParams:

Array -> [params => [username => jake1990], query: []];
Array -> [params => [username => jake1990], query: []];
Array -> [params => [username => jake1990, photoId => 931280134], query: []];
Array -> [params => [username => jake1990, photoId => 931280134], query: [showComments => false]];

---

A few things to point out:

1. I just coded the GET method in the Route class but implementing the other ones (POST, PUT, DELETE) should be easy.

2. I don't know if the code from the routing covers all the cases but I'm trying to achieve that, so if there is a case where it doesn't work and it should, please point it out.

3. If there's any bad practice or antipattern in my code, please tell because I'm striving to follow the best practices to make the code flexible and easy to maintain.

4. It's the first time I do MVC so it's probably full of errors or unnecessary stuff.

Answer 1

Connecting to a database

    private static function connect(){
        $pdo = new PDO('mysql:host='.self::$host.';dbname='.self::$dbName.';charset=utf8', self::$username, self::$password);
        $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        return $pdo;
    }

You call this for every query, so you open a new connection for every query. That's bad, because

1. Connecting is expensive in page load time, memory, and database resources.
2. You never close the connection, so you are relying on the garbage collector to close it. Otherwise, you have a connection for every query.
3. If you run out of connections, no user can connect.

The closest version to what you have would be something like

    private static $pdo = null;

    private static function connect() {
        if (!isset($pdo)) {
            $pdo = new PDO('mysql:host='.self::$host.';dbname='.self::$dbName.';charset=utf8', self::$username, self::$password);
            $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        }

        return $pdo;
    }

This would just make one connection per page load.

It doesn't close the connection manually, but this will do so automatically when the page load ends.

Another common pattern would be to make the connection in index.php for every page load. That may make the connection needlessly if you never use the database, but it saves having to check if the connection exists in every query. For many use patterns, you use the database on every page load anyway.

You could also make the connection in the constructor. Then it will get made as soon as you instantiate the class. Of course, as you have written it, you don't instantiate the class.

HTML context

?>

There are two ways of thinking about this. It could be the match to the open: <?php. Or it switches from PHP context to HTML context. In the latter interpretation, it's unnecessary. You never put HTML after the ?> anyway.

If there are any blank lines after the ?> in a file that runs before any HTML is set, it ends the header section and starts the HTML. This can cause trouble if you then send another header.

Static classes

Static class variables can be problematic in that they prevent you from reusing the code. For example, you can't connect to two different databases without duplicating the code.

Answer 2

Database.php

If you want to maximize the usage of OOP, it might be a good idea to alter this class a bit. Think about 2 years down the line: you suddenly have to create a connection to something other than MySQL. How do you do that? Well, with your code you would have to create a separate database.php file or hack in the functionality into your existing one. But what if there's a better solution? What if we planned for that change now and gave our database class a bit more flexibility?

    class Database{
        protected $host = 'localhost';
        protected $dbName = 'cms';
        protected $username = 'root';
        protected $password = '';

        /* Protected so that derived classes can utilize it (or override it) */
        protected function connect(){
            $pdo = new PDO('mysql:host='. $this->host. 
                  ';dbname='.$this->dbName.';charset=utf8', 
                   $this->username, $this->password);
            $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            return $pdo;
        }

        public function query($query, $params = array()){
            $statement = $this->connect()->prepare($query);
            $statement->execute($params);
            if(explode(' ', $query)[0] == 'SELECT'){
                $data = $statement->fetchAll();
                return $data;
            }
        }

    }

To create a new database class that connects to DB2, you can simply do something like this:

    class DB2 extends Database {
        protected function connect(){
            /* Connect to db2 and return a DB2 handle */
        }
        /* if the db2 library you are using returns an object that 
           has the same interface as the PDO libraries (i.e.: it has
           query(),execute(), and has statements) then your query function
           need not be overridden. Sometimes, that's not the case though,
           so you may have to uncomment the following:

        public function query($sql){
            //Do some db2 specific calls here
        }
        */
    }

Now, if you wanted to replace all instances of where your original Database object was used but instead use your DB2 class, you can (hopefully) only replace it in the code where you did $database = new Database(); and replace it with $database = new DB2();