I created an MVC, am I doing it right?

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
kilbad
Forum Commoner
Posts: 28
Joined: Wed Apr 02, 2008 3:51 pm

I created an MVC, am I doing it right?

Post by kilbad »

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

Code: Select all

 
<?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

Code: Select all

 
<?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

Code: Select all

 
<?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 "N&#259;stase" 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 "N&#259;stase" 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

Code: Select all

 
<?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">&laquo;</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">&raquo;</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') == <!-- s8) --><img src=\"{SMILIES_PATH}/icon_cool.gif\" alt=\"8)\" title=\"Cool\" /><!-- s8) --> {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

Code: Select all

 
<?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;
        }
    }
}
 
?>
 
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: I created an MVC, am I doing it right?

Post by Christopher »

First of all, it is a Controller architecture -- not MVC. I am not saying that as a criticism, you to make sure you are aware that you have both Model and View code in your Controller. That makes it a Transaction Script, which is a fine way to implement things. Just be aware of what you are doing.

Personally, I think the biggest improvement you could make would be moving the Model code (getEntries()) out to it's the Notes class. Less important would be to move more View code out of the Controller.
(#10850)
kilbad
Forum Commoner
Posts: 28
Joined: Wed Apr 02, 2008 3:51 pm

Re: I created an MVC, am I doing it right?

Post by kilbad »

How would you move the getEntries() function out? I have that in there so I do not have to repeat that redundant code twice in the doNotes and doNote functions.

Thanks again for your help!
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: I created an MVC, am I doing it right?

Post by Christopher »

Either load the Model in the constructor or implement a getNotes() or getModel() method that both methods can use to get a Model object.
(#10850)
Post Reply