I created an MVC, am I doing it right?

[cgm225]

I created an MVC class, and recently coded a new notes/blog module for my site.  Everything is working well, but I wanted to get some expert opinions on how I coded everything, in particular, my module (i.e. my NotesActions() class).  Also, overall, have I properly seperated all my logics (i.e. business versus presentation, etc.)?  Am I using the MVC system correctly?  Would you do anything differently?  Any and all feedback is appreciated!

 

Thanks again.. here is the code...

 

My 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 . '/ModelViewController.php5'; //MVC with registry class

//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();

?>

 

My ModelViewController.php5

<?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 MVC root directory path is set from
         * the registry.  This path is needed for the MVC classes to work
         * properly.
         */
        $this->setPageDir();
    }
    
    //Sets the MVC 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;
    }
}

?>

 

My NotesActions.php5

<?php

class NotesActions extends ActionController {
    
    private function getEntries() {
        
        //Sets MySQLi connection object and selects database
        $mysqli = $this->registry->get('mysqli');
        $mysqli->select_db('example_db');
        
        //Includes then instatiates new Notes class
        require_once $this->pageDir . '/' . $this->getModule() . '/' . 'includes/Notes.php5';
        $notes = new Notes($mysqli);
        
        //Gathers all entries from the database and puts them into an array
        $notes->findEntries();
        
        //Returns notes object
        return $notes;
    
    }
    
    public function doNotes() {
        
        //Adds page title to registry
        $this->registry->set('title', 'Example.com &#34;N&#259;stase&#34; edition: Notes');

        //Sets page variable
        $regex = '/[^0-9]++/'; //Only numbers allowed
        $page = isset($_GET['page']) ? preg_replace($regex, '', $_GET['page']) : 1;

        //Returns array of notes
        $notes = $this->getEntries();
        
        //Adds total number of entries to the registry
        $this->registry->set('totalEntries', $notes->getDataCount());        
        
        //Includes/instantiates class to specify number displayed per page
        require_once $this->pageDir . '/NumberDisplayed.php5';
        $numberDisplayed = new NumberDisplayed('notes', 2);
        
        //Adds numberDisplayed per page variable to the registry
        $numberDisplayed = $numberDisplayed->getNumberDisplayed();
        $this->registry->set('notesDisplayed', $numberDisplayed);
        
        //Calculates total number of pages and adds value to the registry
        $totalPages = ceil($notes->getDataCount() / $numberDisplayed);
        $this->registry->set('totalPages', $totalPages);
        
        /* Resets page number to total number of pages if current page value is
         * greater than total pages
         */ 
        $page = ($page > $totalPages) ? $totalPages : $page;
        
        //Adds final page value to the registry
        $this->registry->set('page', $page);

        /* Calculates position of the first and last entry within the data array
         * for any partciular page and adds those values to the registry 
         */
        $firstPageEntry = ($page * $numberDisplayed) - $numberDisplayed;
        $lastPageEntry  = $firstPageEntry + $numberDisplayed;
        $this->registry->set('firstPageEntry ', $firstPageEntry);
        $this->registry->set('lastPageEntry', $lastPageEntry);
       
        //Passes data array to the registry
        $this->registry->set('notes', $notes->getSlicedData($firstPageEntry, $numberDisplayed));

    }
    
    public function doNote() {
        
        //Adds page title to registry
        $this->registry->set('title', 'Example.com &#34;N&#259;stase&#34; edition: Note');
        
        //Sets entry variable and adds to registry
        $regex  = '/[^-_A-z0-9]++/'; //Allowed characters: letter, number, underscore or hyphen
        $entry = isset($_GET['entry']) ? preg_replace($regex, '', $_GET['entry']) : null;
        $this->registry->set('entry', $entry);
        
        //Returns array of notes
        $notes = $this->getEntries();

        //Sets the entry data array into registry
        $this->registry->set('notes', $notes->getData());

    }
}

?>

 

My notesView.php5

<?php

//Ouputs HTML header and openning div
echo "\t\t<div class=\"text_container\">\n";
echo "\t\t\t<h2 class=\"notes\">Notes:: More from me..</h2>\n";
echo "\t\t\tI use this area of my site to share random thoughts, experiences, etc.\n\t\t\t<br />\n\n";

//Ouputs navigation div
echo "\t\t\t<div class=\"note_navigation_container\">\n";
echo "\t\t\t\t<div class=\"note_navigation_right\">\n";

//If there are more pages beyond the current one, a "Next" link is provided
if ($this->registry->get('totalEntries') > $this->registry->get('lastPageEntry')) { 
    echo "\t\t\t\t\t<a href=\"http://" . $_SERVER['SERVER_NAME'] . '/notes/' . ($this->registry->get('page') + 1) . "\">Next</a>\n";
}

echo "\t\t\t\t</div>\n";
echo "\t\t\t\t<div class=\"note_navigation_left\">\n";

//If the user is beyond the first page, a "Previous" link is provided
if ($this->registry->get('page') > 1) {
    echo "\t\t\t\t\t<a href=\"http://" . $_SERVER['SERVER_NAME'] . '/notes/' . ($this->registry->get('page') - 1) . "\">Previous</a>\n";
} else {
    echo "\t\t\t\t\t<span style=\"color:#ffffff;\">_</span>\n";
}

echo "\t\t\t\t</div>\n";
echo "\t\t\t\t<div class=\"note_navigation_center\">\n\t\t\t\t\t";

//Calculating the starting pagination number
$startingPaginationNumber = $this->registry->get('page') - 2;

//If the user is on page four or beyond, a link to page one is provided
if ($startingPaginationNumber <= 1) {
    $startingPaginationNumber = 1;
} else {
    echo '<a href="http://' . $_SERVER['SERVER_NAME'] . '/notes/1" title="First Page">«</a> ';
}

//Calculating the ending pagination number
$endingPaginationNumber = $this->registry->get('page') + 2;

if ($endingPaginationNumber > $this->registry->get('totalPages')) {
    $endingPaginationNumber = $this->registry->get('totalPages');
}

/* Navigation via a numerical index will be provided with links directly to the
* two most adjacent pages on either side of the current page number, as well as
* the additional links to the starting and ending pages,  all such that the
* output will resemble "<< 3 4 5 6 7 >>" where 5 is the current page 
*/
for ($i = $startingPaginationNumber; $i <= $endingPaginationNumber; $i++) {
    if ($i == $this->registry->get('page')) {
        echo '<span style="font-weight:800;text-decoration:underline;">' . $i . '</span> ';
    } else {
        echo '<a href="http://' . $_SERVER['SERVER_NAME'] . "/notes/$i\">$i</a> ";
    }
}

/* If the user is three or more pages from the end, a link to the last page is
* provided
*/ 
if ($endingPaginationNumber < $this->registry->get('totalPages')) {
    echo '<a href="http://' . $_SERVER['SERVER_NAME'] . '/notes/' . $this->registry->get('totalPages') . '" title="Last Page">»</a>';
}

echo "\n\t\t\t\t</div>\n";
echo "\t\t\t</div>\n\n";

//Outputting entry data
foreach($this->registry->get('notes') as $note) {
    echo "\t\t\t<div class=\"notes\">\n";
    echo "\t\t\t\t<h2 class=\"notes\">" . $note['title'] . "</h2>\n";
    echo "\t\t\t\t" . $note['date'] . " | <a href=\"http://" . $_SERVER['SERVER_NAME'] . '/notes/entry/' . $note['id'] . "\">Permalink</a>\n";
    echo "\t\t\t\t<div class=\"notes_body\">\n\t\t\t\t\t" . stripslashes(str_replace("\n" , "\t\t\t\t\t<br />\n\t\t\t\t\t", $note['note'])) . "\n\t\t\t\t</div>";
    echo "\n\t\t\t</div>\n";
}

echo "\t\t</div>";

?>


	<form method="post" action="<?php $_SERVER['PHP_SELF'] ?>" class="notesNumberDisplayed">
	    <div class="notesNumberDisplayed">
                        Display
	        <select name="numberDisplayed" size="1" class="notesNumberDisplayed" onchange="form.submit()">
	            <option value="2" class="notesNumberDisplayed" <?php if ($this->registry->get('notesDisplayed') == 2) {echo "selected=\"selected\"";} ?>>2</option>
	            <option value="4" class="notesNumberDisplayed" <?php if ($this->registry->get('notesDisplayed') == 4) {echo "selected=\"selected\"";} ?>>4</option>
	            <option value="6" class="notesNumberDisplayed" <?php if ($this->registry->get('notesDisplayed') == 6) {echo "selected=\"selected\"";} ?>>6</option>
	            <option value="8" class="notesNumberDisplayed" <?php if ($this->registry->get('notesDisplayed') ==  {echo "selected=\"selected\"";} ?>>8</option>
	            <option value="10" class="notesNumberDisplayed" <?php if ($this->registry->get('notesDisplayed') == 10) {echo "selected=\"selected\"";} ?>>10</option>
	            <option value="20" class="notesNumberDisplayed" <?php if ($this->registry->get('notesDisplayed') == 20) {echo "selected=\"selected\"";} ?>>20</option>
	            <option value="40" class="notesNumberDisplayed" <?php if ($this->registry->get('notesDisplayed') == 40) {echo "selected=\"selected\"";} ?>>40</option>
	        </select>
	        <noscript><p class="no_style">JavaScript required to change the display number!</p></noscript>
                    </div>
	</form>

 

My Notes.php5

<?php

class Notes {
    
    //Declaring variables
    private $connection;
    private $id;
    private $data = array();
    
    //Sets MySQLi object
    public function __construct(mysqli $connection) {
        $this->connection = $connection;
    }
    
    /* Creates a two dimensional array in which entry id numbers are stored in
     * the first dimension, and then for each id number, a second array (i.e.
     * the second dimension) is assigned, which contains all the field values
     * for that particular entry.
     */
    public function findEntries() {
        $query = 'SELECT id, title, note, date, timestamp FROM notes ORDER BY id DESC';
        $statement = $this->connection->prepare($query);
        $statement->bind_result($id, $title, $note, $date, $timestamp);
        $statement->execute();
        while($statement->fetch()) {
            $this->data[$id] = array(
                'id'         => $id,
                'title'      => $title,
                'date'       => $date,
                'note'       => $note,
                'timestamp'  => $timestamp
            );
        }
        $statement->close();
    }
    
    //Returns the data array
    public function getData() {
        if (!empty($this->data)){
            return $this->data;
        } else {
            return false;
        }
    }
    
    //Returns the number of items in the data array
    public function getDataCount() {
        if (!empty($this->data)){
            return count($this->data);
        } else {
            return false;
        }
    }
    
    //Returns a particular range of the data from the data array
    public function getSlicedData($offset = 0, $length = 10000) {
        if (!empty($this->data)){
            return array_slice(array_values($this->data), $offset, $length);
        } else {
            return false;
        }
    }
}

?>

[ignace]

quite impressive you nicely took my advice except for separating the model from the controller. The view actually is some sort of template engine, where it includes an html page and replaces values within the html with internal values usually coming from models.

 

I also see you have done a read up on patterns (Registry pattern in particular) nice job!

[cgm225]

Thanks for your help.  Following up, specifically how should I be better separating the model from the controller? 

 

Also, I suppose the view is a very simple template engine as you have described... is that ok?  Again, I am new to OOP and MVC, so I want to stay as true to the MVC system as possible, but also to keep everything as simple as possible (that is a major goal of mine).  In general I have tried to separate all my different logics, but what can I improve on?

[ignace]

hope this helps as long you keep your view solely for rendering, your model solely for fetching and manipulating data and your controller only has possible user actions defined you should be ok!

 

class View {
     function __set($key, $value) {}
     function __get($key) {}
     function __isset($key) {}
     function __call($helper, $args) {}
}

abstract class ActionController {
     var $view;
     function init() {
           $this->view = new View();
     }
}

// assume ?model=foo&controller=bar&action=view&id=1
// or with mod_rewrite /foo/bar/view/id/1
class FrontController extends ActionController {
    function dispatch($action, $controller, $module = null, array $params = array()) {
         if (!$module) {
             $module = '<default-module-maybe-specified-in-your-config-file-fetch-using-the-registry>';
             // or just: $module = 'default';
         }
         // open modules directory, open module $module directory, search for class $controllerController.php and create an instance of it
         // if module is specified, then prefix controller using the module name
         $controllerName = ucfirst($controller) . "Controller";
         require_once $modulePath . DIRECTORY_SEPARATOR . $controllerName . ".php";
         if ($module)
              $controllerName = ucfirst($module) . "_" . $controllerName;
         if (!class_exists($controllerName)) {
                  // file loaded but Module_ControllerController() was not declared inside the file
         }
         $controller = new $controllerName();
         $actionName = $action . "Action";
         if (method_exists($actionName, $controller)) {
               $controller->$actionName($params);
         }
    }
}

// example controller
class BlogController extends ActionController {
      var $model;
      function init() {
           $this->model = new BlogsModel(); // BlogsModel is a table data gateway pattern
           parent::init();
      }
      
      function editAction(array $requestParams = array()) {
            $id = (int) $requestParams['id'];
            $row = $this->model->findRow($id);
            if (0 !== sizeof($row)) {
                  // provide data for rendering in /blog/edit.html
                  $this->view->rowData = $row;
            }
        }
}

// /blog/edit.html
<!-- file is included within the view object use it's functionality -->
<?php echo $this->render('head.html'); ?>

<h1>Editing: "<?php echo $this->rowData['title']; ?>"</h1>
.. form with populated input fields ..

<?php echo $this->render('foot.html'); ?>