php - Creating an MVC using good practices, and making it testable

343

I recently asked my girlfriend to teach me how to create a website using PHP and if possible to make me understand frameworks. “Of course” she replied helpfully, and gave me some links and books to help me understand the basics of how it works, and what PHP is.

In my web-grazing I visited many sites, including this one (which appeared whenever I had a serious question not relating to cats) and learned about many things including, but not limited to, classes, autoloading, database connections, evil singletons, and design patterns.. not all of which I totally grasped, but at least I wasn’t like ’uh, what?’ when she mentioned them.

We then sat down and she walked me through the getting started phase of creating my own MVC framework.

We created the following folder structure: (abridged):

app/core/

app/controllers/template/

and routed all requests via index.php, to the router class and on the way loaded an autoload class that registered the classes we will use on the site (currently only Template - in controllers/template/template.php - which only echos ‘Hello Rufferto’)

It was at this point that she told me that she liked to create a router table in the database, rather than keeping the routing info in a file, and said:

“let’s instantiate a database class in the router class where we will read the table info. I have my database class already in the app/core directory, It’s got all the mysql_connect() stuff already, and we only ever need one connection, so we’ll load it here as a singleton."

I of course immediately grabbed all her stuff, and threw it and her out into the street, shouting "Don't come back you evil harridan! I think the people at stackoverflow will have to take it from here’

This does leave me with a small problem. Everything we had done up to that point was now being called into question, and before I go any further with my plan to create an MVC framework that will end world hunger, create peace throughout the land, and make anyone who helps me with this question totally irresistible to every member of the gender they prefer, I want to make sure I haven’t started off with all the wrong ingredients.

I have used my most newly acquired skill - pasting - to put the code so far below, and wish some help with the black clouds hanging over the code, which I have put, as I see them, below said code....

index.php:

<?php
require_once(__DIR__ . '/app/config.php');
\APP\Core\Router::create();

config.php:

require_once 'core/autoloader.php';

autoloader.php:

<?php
namespace APP\Core;

class Autoloader
{
public static $loader;

public static $namespaces = array(
    'APP\Core' => array('app/core'),
    'APP\Models' => array('app/models'),
    'APP\Controllers' => array('app/controllers')
);

public static function init()
{
    if (self::$loader == null) {
        self::$loader = new self();
    }
    return self::$loader;
}

public function __construct()
{
    spl_autoload_register(function ($class_name) {

        foreach (Autoloader::$namespaces as $current_namespace => $paths) {
            //iterate through each namespace and add the classes - code removed for brevity - it working fine!
        }
    });
}
}
Autoloader::init();

router.php:

<?php
namespace APP\Core;

class Router
{
function __construct()
{
    // crazy ex girfriend instantiated databse here, ive moved it since i want to use PDO
}

public static function create($path='')
{
    $cl=get_called_class();
    $class = new $cl;

    if(empty($path)) $path = parse_url($_SERVER['REQUEST_URI'], PHP_URL_PATH);
    $query = parse_url($_SERVER['REQUEST_URI'], PHP_URL_QUERY);

    $db_opt = [
        \PDO::ATTR_ERRMODE            => \PDO::ERRMODE_EXCEPTION,
        \PDO::ATTR_DEFAULT_FETCH_MODE => \PDO::FETCH_ASSOC,
        \PDO::ATTR_EMULATE_PREPARES   => false,
    ];

    $db = new \PDO(STD_PDO_DSN, DB_USER, DB_PASSWD, $db_opt);
    $sql = "SELECT * FROM `router` where (regex=1 and ? REGEXP `source`) or (regex=0 and `source` = ?) order by `order` asc";
    $stmt = $db->prepare($sql);
    $stmt->execute([$path, $path]);
    $redirect = $stmt->fetch();
    if($redirect){
        // code here just makes params array from the table - stuff like a body class, meta title etc
            $class_redirect::create($params)->run();
        }
    }else{
        echo 'you're an idiot, and should just flip burgers instead';
    }
}
}

template.php:

<?php
namespace APP\Controllers\Template;

class Template
{
public function process_some_action()
{
    // Like dealing with a post containing new baby names
}

public function process_default()
{
    // Do incredible default stuff like list baby names
}

public static function create($params=array())
{
    $class = get_called_class();
    return new $class($params);
}

function run($params=array())
{
    $processed=false;

    $processed=$this->call_if_exists("process_some_action");

    if(!$processed){
        $processed=$this->call_if_exists("process_default");
    }

    if($processed){
        $this->view();
    } else {
        echo 'Oh ffs Groo';
    }
}

function call_if_exists($method_name)
{
    if(method_exists($this, $method_name)){
        $this->$method_name();
        return true;
    } else {
        return false;
    }
}

function view()
{
    echo '<br/>';
    echo 'Hello Rufferto!';
    echo '<br/>';
}
}

1: am I calling the autoloader correctly - is it ok to put it in the ‘config’ file, or should I be putting it somewhere else?

2: er, is it a singleton? If so how can I fix that?

3: I want to use PDO, and have seen a couple of posts directing the questioner to ‘get rid of the useless database class…. But surely what I am looking for here is a ‘class’ that contains the code to make a connection, and have all the controllers or models that need it just call it (and if it doesn’t exist already create it)?

4: I am not convinced I understand the point of the 'create()' function in template (or router for that matter)....

Of course there may be other things wrong, and feel free to point them out to me, but these 4 things are the root of my issue, as I have been going round in circles, renaming things, getting into a state because I’m no longer sure if things are singletons, calling classes that wrap the PDO stuff, then scrapping it all and starting again.

No, there’s no models or views …. I’m not there yet (I will almost certainly use xslt unless the internet breaks with all the stackers shouting at me that it’s moronic) and 1 last thing…… I am not yet comfortable enough with my understanding of dependency injection to use it in any sentence other than this one.

P.S. Absolutely the 1 thing I will never do is call the ex and tell her I’m stuck…….

P.P.S If you have got this far, here’s a treat: http://img1.joyreactor.com/pics/post/comics-threepanelsoul-programmer-job-653145.png

661

Answer

Solution:

This question would be better suited for codereview. SO is more for "this code doesn't work, what did I do wrong?" questions, rather than "this code works, but can it be improved or am I even doing the right thing?" type questions.

Having said that:

  1. Usually, config files are just that: files containing configuration details. Think database credentials, API keys for third party resources, that sort of thing. You should NOT instantiate classes there. Most full frameworks include a Bootstrap class that handles that sort of thing: initialising the application, loading the config file, autoloader and routes, etc.
  2. Your autoloader is currently a static class with only static methods. That's one way of making a singleton. There's nothing really wrong with that, because you'll only ever want one autoloader in your application.
  3. I suggest you look into some ORM-type libraries, like Doctrine. I've just started work on a project that uses CakePHP's ORM library. If you want to build it yourself, I recommend writing an abstract base model class that has all the logic for retrieving, saving and deleting entries from the database, where each model class itself has a few properties that are unique per model (such as the database table name, for example) and extends this base model.
  4. Those::create() functions would be useful in a singleton, where they return the existing object if it has already run or instantiate a new one if not, (see example below). Since you seem to have a dislike for singletons, you might want to think about dropping thecreate() method and just doing the initial setup in the constructor.

Additionally, I wouldn't recommend XSLT for templating. It's a pain, because you'll have to make sure all your controllers build an XML file for your content. If you're going to use templates, look into existing template engines like Twig or Smarty.

Here's that example I promised about using acreate() function for a singleton:

class Template {
    private static $instance;

    public static function create($params = array()) {
        if (self::$instance !== null) {
            return self::$instance;
        }

        self::$instance = new static($params);
        // do additional setup for the new instance here
        return self::$instance;
    }
}

Although I'd personally use the nameget() instead ofcreate().

People are also looking for solutions to the problem: php - How to send data without reloading page with cURL?

Source

Didn't find the answer?

Our community is visited by hundreds of web development professionals every day. Ask your question and get a quick answer for free.

Ask a Question

Write quick answer

Do you know the answer to this question? Write a quick response to it. With your help, we will make our community stronger.

Similar questions

Find the answer in similar questions on our website.