MVC application class

Question

Please could you review and critic my code and logic? Is this is along the right lines of an MVC application class or not?

namespace dh_mvc2;

    use dh_mvc2\dispatchers\Base_Dispatcher;
    use dh_mvc2\routers\Base_Router;
    use dh_mvc2\autoloaders\Basic_Autoloader;
    use dh_mvc2\classes\Config;

    class Application {
    protected $_config;
    protected $_autoloader;
    protected $_app_path;
    protected $_framework_path;
    protected $_router;
    protected $_dispatcher;

    /**
     *
     * @param string $app_path
     *          "path/to/application/directory";
     * @throws \DH_MVC2_Application_Exeption
     */
    public function __construct($app_path, $run = TRUE) {
        if (! is_dir ( $app_path )) {
            require_once 'exeptions/DH_MVC2_Application_Exeption.php';
            throw new \DH_MVC2_Application_Exeption ( "Must supply a path to an application directory" );
        }

        $this->_framework_path = realpath ( dirname ( __DIR__ ) );
        $this->_app_path = $app_path;
        $this->set_ini_default_paths ();
        $this->init ();

        if ($run) {
            $this->run ();
        }
    }

    /**
     * add the framework dir and application dir to include path
     */
    private function set_ini_default_paths() {
        $paths = explode ( PATH_SEPARATOR, get_include_path () );
        if (array_search ( $this->_app_path, $paths ) === false) {
            array_push ( $paths, $this->_app_path );
        }
        if (array_search ( $this->_framework_path, $paths ) === false) {
            array_push ( $paths, $this->_framework_path );
        }
        set_include_path ( implode ( PATH_SEPARATOR, $paths ) );
        spl_autoload_register ();
    }

    /**
     * links to the app and framework default config files and inits them
     * registers autoloader
     */
    private function init() {
        $config_file = '/config/config.ini.php';
        $config_files ['DH_MVC'] = __DIR__ . $config_file; //framework config
        $config_files ['APP'] = $this->_app_path . $config_file; //app config

        $this->_config = new Config ( $config_files );
        $config_paths = $this->_config->get_paths ();
        $this->_autoloader = new Basic_Autoloader ( $config_paths );
    }

    /**
     * creates new base router instance which reads the url request and routes
     * application onto Action Controllers
     */
    public function run() {
        $this->_router = new Base_router ( $this->_config );
        $route = $this->_router->run ();

        if ($route) {
            $this->_dispatcher = new Base_Dispatcher ( $route );
            $this->_dispatcher->run ();
        }
    }
}

Not moved the initialization of new router to construct in order to remove protected $_config as I might use this attribute depending on how I develop the class. If not used I will remove.

$config_files ['DH_MVC'] = __DIR__ . $config_file;

It uses __DIR__ as $_framework_path is parent of DIR. $_framework_path takes into account my namespace dh_mvc2 thats used as the first directory, by the spl_autoload, from the framework basepath

namespace dh_mvc2\application;

    use dh_mvc2\classes\Config;
    use dh_mvc2\dispatchers\Base_Dispatcher;
    use dh_mvc2\routers\Base_Router;
    use dh_mvc2\autoloaders\Basic_Autoloader;

    class Application {
    protected $_config;
    protected $_autoloader;
    protected $_app_path;
    protected $_framework_path;
    protected $_router;
    protected $_dispatcher;

    /**
     *
     * @param string $app_path
     *          "path/to/application/directory";
     * @throws \DH_MVC2_Application_Exeption
     */
    public function __construct($app_path, $run = TRUE) {
        if (! is_dir ( $app_path )) {
            require_once 'exeptions/DH_MVC2_Application_Exeption.php';
            throw new \DH_MVC2_Application_Exeption ( "Must supply a path to an application directory" );
        }

        $this->_framework_path = realpath ( dirname ( __DIR__ ) );
        $this->_app_path = $app_path;
        $this->set_ini_default_paths ();
        $this->init ();
        $d = new Config ();
        if ($run) {
            $this->run ();
        }
    }

    /**
     * add the framework dir and application dir to include path
     */
    private function set_ini_default_paths() {
        spl_autoload_register ();
        $paths = explode ( PATH_SEPARATOR, get_include_path () );
        array_push ( $paths, $this->_app_path );
        array_push ( $paths, $this->_framework_path );
        $paths = array_unique ( $paths );
        set_include_path ( implode ( PATH_SEPARATOR, $paths ) );
    }

    /**
     * links to the app and framework default config files and inits them
     * registers autoloader
     */
    private function init() {
        $config_file = '/config/config.ini.php';
        $config_files ['DH_MVC'] = __DIR__ . $config_file; // framework config
        $config_files ['APP'] = $this->_app_path . $config_file; // app config
        $this->_config = new Config ( $config_files );
        $config_paths = $this->_config->get_paths ();
        $this->_autoloader = new Basic_Autoloader ( $config_paths );
    }

    /**
     * creates new base router instance which reads the url request and routes
     * application onto Action Controllers
     */
    public function run() {
        $this->_router = new Base_router ( $this->_config );
        $route = $this->_router->run ();

        if ($route) {
            $this->_dispatcher = new Base_Dispatcher ( $route );
            $this->_dispatcher->run ();
        } else {
            require_once 'exeptions/DH_MVC2_Router_Exeption.php';
            throw new \DH_MVC2_Router_Exeption ( "Failed to discover route" );
        }
    }
}

Answer 1

this looks pretty good to me.

Some thoughts for entertainment;

• If you created the router in init(), then _config would not be required any more as a protected property, one less private property

• There seems to be no error handling when $this->_router->run () fails.

• set_ini_default_paths violates DRY, but only once, have you considered just adding the paths and then calling array_unique. Not sure that is the right way to go, but it could be considered.

• _autoloader is nowhere used, and it is private. Seems pointless?

• Should $config_files ['DH_MVC'] = __DIR__ . $config_file; not use $this->_framework_path ?

Answer 2

I have some criticism on details you didn't ask, but which should be addressed:

Autoloading

Make use of it! There is no need to make manual calls to require_once().

There even is a standard: PSR-0, which defines how you should structure your class and namespace names together with the directory and file structure. When I look at your code, I see some issues: Because the underscore was used as a separator before PHP got namespaces, it is considered a separation character in class names.

Namespaces

Make use of it! I cannot see a reason why the exception \DH_MVC2_Application_Exeption is located in the root namespace, and not called \dh_mvc2\application\Exception. It would be so much easier to simply throw new Exception(); inside the "\dh_mvc2\application" namespace.

Whitespace and coding style

Endless debates might occur, but I really don't like yours. Especially the inconsistent placement of parentheses. Personally, I'd rather prefer not to use that much spaces, but if you really have to, use them everywhere. For example, if you want to find the function "set_ini_default_paths", and want to make sure not to find "set_ini_default_paths_directory", you'd search for "set_ini_default_paths(" - which will find only function definitions, but not usage. To find these, you'd have to search "set_ini_default_paths (".

Dependency injection

Doesn't take place. Objects are created inside your class. There is no way I would be able to change for example the Config object if I'd use your class, I must use yours. I even cannot change the config filename!

Include path

Be careful what you add here. If a PSR-0 autoloader is used, there is no need to add anything to the include path. In fact, you'll get a good amount of performance if you include as few directories as possible, preferably only ".", to be able to include files with a relative path.