Looking for feedback on controller class... NEW to OOP

[cgm225]

I am new to OOP, and wanted to get some feedback on my controller class that I use to run my personal website.  Any constructive criticism would be great!  My overall goal was to code it well, but to keep it simple too.

 

 

<?php

class FrontController extends ActionController {

    //Declaring variable(s)
    private static $instance;
    protected $controller;

    //Class construct method
    public function __construct() {}

    //Starts new instance of this class with a singleton pattern
    public static function getInstance() {
        if(!self::$instance) {
            self::$instance = new self();
        }
        return self::$instance;
    }

    public function dispatch($throwExceptions = false) {
        
        /* Checks for the GET variables $module and $action, and, if present,
         * strips them down with a regular expression function with a white
         * list of allowed characters, removing anything that is not a letter,
         * number, underscore or hyphen.
         */
        $regex  = '/[^-_A-z0-9]++/';
        $module = isset($_GET['module']) ? preg_replace($regex, '', $_GET['module']) : 'home';
        $action = isset($_GET['action']) ? preg_replace($regex, '', $_GET['action']) : 'frontpage';

        /* Generates Actions class filename (example: HomeActions) and path to
         * that class (example: home/HomeActions.php5), checks if $file is a
         * valid file, and then, if so, requires that file.
         */
        $class = ucfirst($module) . 'Actions';
        $file  = $this->pageDir . '/' . $module . '/' . $class . '.php5';

        if (!is_file($file)) {
            throw new FrontControllerException('Page not found!');
        }

        require_once $file;

        /* Creates a new instance of the Actions class (example: $controller
         * = new HomeActions(), and passes the registry variable to the
         * ActionController class.
         */
        $controller = new $class();
        $controller->setRegistry($this->registry);

        try {
            //Trys the setModule method in the ActionController class
            $controller->setModule($module);

            /* The ActionController dispatchAction method checks if the method
             * exists, then runs the displayView function in the
             * ActionController class.
             */    
            $controller->dispatchAction($action);
        }
        catch(Exception $error) {

            /* An exception has occurred, and will be displayed if
             * $throwExceptions is set to true.
             */
            if($throwExceptions) {
                echo $error->errorMessage($error); //Full exception echoed
            } else {
                echo $error->errorMessage(null); //Simple error messaged echoed
            }
        }
    }
}

abstract class ActionController {

    //Declaring variable(s)
    protected $registry;
    protected $module;
    protected $registryItems = array();
    
    //Class construct method
    public function __construct(){}
    
    public function setRegistry($registry) {
      
        //Sets the registry object
        $this->registry = $registry;
        
        /* Once the registry is loaded, the controller root directory path is
         * set from the registry.  This path is needed for the controller
         * classes to work properly.
         */
        $this->setPageDir();
    }
    
    //Sets the controller root directory from the value stored in the registry
    public function setPageDir() {
        $this->pageDir = $this->registry->get('pageDir');
    }

    //Sets the module
    public function setModule($module) {
        $this->module = $module;
    }

    //Gets the module
    public function getModule() {
        return $this->module;
    }

    /* Checks for actionMethod in the Actions class (example: doFrontpage()
     * within home/HomeActions.php5) with the method_exists function and, if
     * present, the actionMethod and displayView functions are executed.
     */  
    public function dispatchAction($action) {
        $actionMethod = 'do' . ucfirst($action);
        if (!method_exists($this, $actionMethod)) {
            throw new FrontControllerException('Page not found!');
        }
        $this->$actionMethod();
        $this->displayView($action);
    }

    public function displayView($action) {
        if (!is_file($this->pageDir . '/' . $this->getModule() . '/' . $action . 'View.php5')) {
            throw new FrontControllerException('Page not found!');
        }

        //Sets $this->actionView to the path of the action View file
        $this->actionView = $this->pageDir . '/' . $this->getModule() . '/' . $action . 'View.php5';

        //Sets path of the action View file into the registry
        $this->registry->set('actionView', $this->actionView);

        //Includes template file within which the action View file is included
        require_once $this->pageDir . '/default.tpl';
    }
}

class Registry {
    
    //Declaring variables
    private $store;

    //Class constructor
    public function __construct() {}
    
    //Sets registry variable
    public function set($label, $object) {
        $this->store[$label] = $object;
    }
   
    //Gets registry variable    
    public function get($label) {
        if(isset($this->store[$label])) {
            return $this->store[$label];
        }
        return false;
    }
    
    //Adds outside array of registry values to $this->store array
    public function addRegistryArray($registryItems) {
        foreach ($registryItems as $key => $value) {
            $this->set($key, $value);
        }
    }
    
    //Returns registry array
    public function getRegistryArray() {
        return $this->store;
    }
}

class FrontControllerException extends Exception {
    public function errorMessage($error) {
        
        //If and throwExceptions is true, then the full exception is returned.
        $errorMessage = isset($error) ? $error : $this->getMessage();
        return $errorMessage;
    }
}

?>

 

 

Example of implementation in index.php5

<?php

//Initializing session
session_start();

//General configurations
require_once 'configurations.php5';                 

//Instatiating MySQLi object for all modules with NO database selected.
$mysqli = new mysqli(MYSQL_SERVER, MYSQL_SERVER_USERNAME, MYSQL_SERVER_PASSWORD);

//Additional includes
require_once MVC_ROOT . '/Authentication.php5';      //User authentication
require_once MVC_ROOT . '/RegistryItems.php5';       //Default registry items
require_once MVC_ROOT . '/ControlArchitecture.php5'; //Controller with registry

//Instantiating registry object
$registry = new Registry();
$registry->addRegistryArray($registryItems);

//Instantiating front controller object
$controller = FrontController::getInstance();
$controller->setRegistry($registry);              
$controller->dispatch(false);

//Closing MySQLi object
$mysqli->close();

?>

[aschk]

Having had a skim, a couple of things:

 

1) If you're going to make a singleton then make sure to make the __construct() final (by using the final keyword), and also override the __clone() method

 

2) Consider creating some protected variables inside the FrontController for $_defaultModule, and $_defaultAction and use them in your dispatch() method instead of 'home' and 'frontpage' (i.e. $this->_defaultModule and $this->_defaultAction). This way they can be overridden by subsequent child subclasses.

 

3) More of question really; shouldn't the registry be a singleton also? Thus you don't require an unnecessary reference inside the abstract controller to it, and you don't have to pass it to subsequent controller classes... Do you really need more than 1 instance of registry?