Implementation of MVC Bootstrap & Factory

Question

Before I get started, I feel I need to list the following.

• I am not looking for an existing DI or Framework this is a personal project to shift from procedural to oop programming.

• Tear this apart. I am trying to learn the latest best practices i do not want to be stuck creating broken code for future builds/projects.

I recently posted a question regarding a static registry to access site-wide dependencies across the site. Since posting that 2 days ago I have begun to look into dependency injection thanks to the comments I received. I understand the idea of DI (passing object/var dependencies through constructor setter). The issue I've had was understanding how the application should be structured in order to allow for this. But I feel I have a bit of a grasp on how things should work. I am looking for feedback on my app bootstrap/init file, as well as the Factory/DI Container.

I have the standard 'gateway' index file within root that launches the application bootstrap file.

This is what the current bootstrap file looks like:

namespace Core;

class Bootstrap {
    private $factory;

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


    /**
     *  BUILD APPLICATION
     *
     *  Assign Core Classes + Site Configuration
     *  Autoload Vars, Array's, & Classes Within            ROOT . 'application/config/autoload/'
     *  Instantiate Router & Set Routes
     */
    public function build() {  
        $db         = $this->factory->make('\Helper\Database');
        $this->factory->set('db', $db);

        $site       = $this->factory->make('\Core\Config')->set();
        $this->factory->set('vars', ['site' => $site]);

        $session    = $this->factory->make('\Helper\Session');
        $session->start();


        $autoload = $this->autoload($site);
        foreach ($autoload as $key => $value) {
            $this->factory->set($key, $value);
        }


        require CONFIG.'routes.php';
        $this->router   = $this->factory->make('\Core\Router', [
            'subdomain' => $site['subdomain'],
            'platforms' => $site['platforms'],
            'closed'    => $site['closed']
        ]);
        $this->router->setRoutes($routes);
    }


    /**
     *  LAUNCH APPLICATION
     *
     *  $route              Map Route Request & Return      Controller, Method, Params As Assoc Array
     *
     *  $controller         Instantiate Controller Within Factory & Pass Dependencies ( Via Factory )
     *  @return             Call Assigned Controller Method & Pass Params
     */
    public function launch() {
        $route      = $this->router->map();

        $controller = $this->factory->make('\Controller\\'.$route['controller']);
        return call_user_func_array([$controller, $route['method']], $route['params']);
    }


    /**
     *  Autoload Vars, Arrays, & Classes Within             ROOT . 'application/config/autoload/'
     *
     *  @param  array       Site Configuration Var Needs to Be Within Var Key
     *
     *  @return array       Vars, Array's, & Instantiate Classes
     */
    public function autoload($site) {
        $return = [];                                                  

        if (file_exists(AUTOLOAD . 'vars.php'))     {   require_once AUTOLOAD . 'vars.php';       }
        if (file_exists(AUTOLOAD . 'classes.php'))  {   require_once AUTOLOAD . 'classes.php';    }

        if ($var)   {    $return['vars'] = array_merge($var, ['site' => $site]);    }
        if ($class) {
            foreach ($class as $key => $value) { 
                $return[$key]       = $value;
            }
        }

        return $return;
    }
}

I use a namespace autoloader that is set within the index. The Factory is called then injected through bootstrap constructor. Most of it is self explanatory,

$site contains site related config that is pulled from db such as default timezone, site email address, site social media, etc.

Sessions are handled via a set_session_handler and are saved within the db.

The autoload idea was taken from the CodeIgnitor framework, which makes things easier with site wide data that is the same:

Var   Example timezones, month, day, year, reserved/restricted usernames, etc. 

The ExampleFormAuthService (needs to load before router, router displays ban page if user is banned, or allows user to see site even when it is 'closed' if user is staff). I was told I should set this as a dependency in each class, but I felt this would be redundant since the entire site is user driven and needs to know if user is logged in.

Router is not /controller/method/id based. I don't like the idea of doing that, plus most URLs needed do not fit that pattern, so routes are manually defined. Routes go through a static route check (index, login, etc.) and if a controller is not found then dynamic route is looped through using preg_match plus regex to define/find route. If a route is not found the 404 page is set.

The route is returned to bootstrap which then instantiates the controller through factory passing dependencies.

namespace Core;

class Factory {
    private $container  = [];
    private $cache      = [];


    public function set($key, $value) {
        $this->container[$key] = $value;   
    }


    public function get($key) {
        if ($this->exists($key)) {      
            return $this->container[$key];
        }
        return '';
    }


    public function make($classname, $params = []) {        
        if (!$this->exists($classname, 'cache')) {
            if (!$params) {
                $dependencies   = $this->dependencies($classname);
                $params         = array_merge($this->class($dependencies['classes']), $this->vars($dependencies['vars']));
            }
            $this->cache[$classname] = new $classname($params);
        }
        return $this->cache[$classname];
    }


    private function exists($key, $container = 'container') {
        return array_key_exists($key, $this->$container);
    }


    private function dependencies($classname) {
        $classes        = array_reverse(class_parents($classname));
        $classes[]      = $classname;

        $dependencies   = [];
        $vars           = [];

        foreach ($classes as $c) {
            $classvars  = get_class_vars($c);

            if (array_key_exists('dependencies', $classvars)) {
                $dependencies = array_merge($dependencies, $classvars['dependencies']);
            }
        }

        if (array_key_exists('vars', $dependencies)) {
            $vars = $dependencies['vars'];
            unset($dependencies['vars']);
        }

        return [
            'classes'   => $dependencies,
            'vars'      => $vars
        ];
    }


    private function classes($classes = []) {
        foreach ($classes as $key => $class) {
            if ($this->exists($key))                {    $value = $this->container[$key];        }
            elseif ($this->exists($class, 'cache')) {    $value = $this->cache[$class];          }
            else {
                $clist  = $this->dependencies($class);   

                if (in_array($clist[$classname]))   {    $this->shutdown($classname, $class);    }
                else {                                   $value = $this->make($class);           }
            }

            $classes[$key] = $value;
        }

        return $classes;
    }


    private function vars($vars = []) {
        foreach ($list['vars'] as $key => $vars) {
            if (array_key_exists($key, $this->container['vars'])) {
                $var    = $this->container['vars'][$key];
                $value  = [];

                if (!$vars) {   $value = $var;   }
                else {
                    foreach ($vars as $v) {         
                        $value[$v] = isset($var[$v]) ? $var[$v] : '';             
                    }
                }   
                $vars[$key] = $value;
            }
        }   

        return $vars;
    }


    private function shutdown($classname, $class) {
        if (ENVIRONMENT == 'development') {
            $msg = 'It Seems The Factory Caught a Dependency Loop When Creating Class ' . 
            $classname . ' AND ' . $class . '. Dependency Loops Cause a Never Ending Chain of the Factory Creation Process! FIX IT!';
        }
        else {  
            $msg = 'It Appears an Error Has Occured, Please Contact a Site Administrator As Soon As Possible, In Order to Fix This Issue';  
        }

        require VIEWS . 'emergency.php';
        exit;
    }


    public  function __construct()  {}
    private function __clone()      {}
    private function __wakeup()     {}
}

Similar were ideas taken from pimple/dice DI (cache) instead of using reflection classes, I use a publicly set $dependency var, models are easy to DI for all they need is the $db class while controllers need, input sanitizer, form builder (certain pages have dynamic forms), view/template creator, flash messages helper, etc. I didn't want to deal with setting all of these within construct and adding a controller wide dependency leaving me with the task of altering the construct of all controllers.

Instead I will use things like:

class BaseControllerExample {
    public $dependencies = [
        'input'  => '\Helper\Input',
        'overwrite' => 'example',
        etc.
    ];

    protected $input,
              $overwrite;
}

class IndexController extends BaseControllerExample {
    public $dependencies = [
        'overwrite' => 'example2'
    ];
    protected $overwrite;
}

To clarify, I also did not want to set individual dependencies for each controller, etc. through bootstrap closures, etc. Overall I see this as being much easier since I can set parent classes with default dependencies for models, controllers, etc. and I can then add specifically defined dependencies within the controller itself if needed. Also, factory is created in a way to overwrite the parent dependency so there is no need to worry about parent default dependencies overwriting the child dependencies.

When using the factory the container and cache is separated to allow the use of the same class multiple times. For example the DB has predefined credentials that are defined as constants for the default db. So in bootstrap default db is loaded. But down the line I want to work on a custom metrics plugin etc. Let's say I want to use a separate db for that.

All I have to do is set the following:

public $dependencies = [
    'mdb' => '\Helper\Database'
];

The factory will pull class from cache instead of pulling $db (default db) from container, and I will be able to use $mdb within the class I set or within app if I pass within bootstrap, and all I have to do to set correct metrics db is $mdb->credentials([assoc array w/ credentials]);.

To set dependencies within classes, I use a foreach loop to assign the vars for use within the class. I have a base class that only serves this purpose. $this->dependencies Keys are set as the var, while the value is the class.

namespace Helper;

class Base {
    public function __construct($params = []) {
        foreach ($params as $key => $value) {
            $this->$key = $value;
        }
    }
}

For classes with only a few dependencies, I manually set dependencies within the class to avoid extending the base class, with classes like controllers that use the same dependencies I extend the base class. If I need to run something within constructor I just ensure I am calling parent::__construct(); and I pass the params on through. I know it would be better to set vars based on the class dependencies. I'm iterating through `$this->dependencies' but I felt that it was extra processing for nothing. If the factory passes other data then it has an error that needs to be fixed.

It is bad practice to have a dependency loop and I do not have one but I figured in the event that a mistake is made, I should catch the error and produce a user friendly error if the site is in production?

What do you all think? Are things setup in a proper way? (MVC implementation) I have the controller, model and view stuff down (inquired about that before). Right now is just the change of the registry -> factory. Am I breaking some sort of fundamental rule or using an anti-pattern? Am I confusing or misunderstanding the DI concept? Is there something that can be done differently or cleaner?

With the factory this is the 3rd iteration (1st non static registry iteration), I am currently thinking of replacing the $container with an assoc $key List that just references the class within the cache.

Answer 1

Looks quite good to me, however i have a problem understanding this part:

public $dependencies = [
    'input'  => '\Helper\Input',
    'overwrite' => 'example',
    etc.
];

protected $input,
          $overwrite;

Why do you need the dependencies global per controller, and not load them per controller action. My idea was that every time you need to load a dependency you would use something like:

$this->get(overwrite)->blabla()

Now if you need to call overwrite many times in the same request that's fine, since the get() is cached anyways. All the information about the mapping should be in some sort of metadata config and not part of the controller, so code like 'input' => '\Helper\Input',, is kinda against DI way of thinking because now, there is a hardcoded dependency on Helper\Input in your controller, which means that if some time down the road you decide to change the input class, you need to go to every place you specified it.

My advice:

1) Kill this public $dependencies = [ 'mdb' => '\Helper\Database' ]; instead explicitly load any dependencies with $this->get('dep')->something, if you need it to be a class member, you can always instantiate in the constructor and share the instance (although again, since its cached its not really a big improvement).

2) Move this public $dependencies = [ 'mdb' => '\Helper\Database' ]; into a big array somewhere into your config, dont make the mapping part of the implementation, by defining the dependency resolutions in the controller. The same dependency SHOUDLN'T be mapped to different classes at runtime anyways.

Hope that helps, the rest of the code looks solid to me, actually quite nice.