2
\$\begingroup\$

I'm currently developing a music website using OOP PHP and I'm trying to correctly implement the Model View Controller pattern.

I am creating this website from scratch so I would like to avoid answers suggesting I use frameworks such CakePHP etc please.

Here are simplified snippets of the files I'm using to generate a page on my site that displays an album release.

albumView.php (VIEW)

<?php
include_once($_SERVER['DOCUMENT_ROOT'].'/../app/config.php');
include('commonHead.php'); //shared site head
include('htmlHeader.php'); //shared website header
$album = new AlbumController(); //instantiate controller
?>
<div id="main">
    <article>
        <header>
            <h1><?php $album->echoArtist(); ?></h1>
        </header>
        <?php if($album->isTracklisting()) : ?>
            <section id="tracklisting">
                <header>
                    <h1>TRACK LISTING</h1>
                </header>
                <ol>
                    <?php $album->echoTracklisting() ?>
                </ol>
            </section>
        <?php endif ?>

...etc...

<?php
include('htmlFooter.php');
include('commonFoot.php');
?>

AlbumController.php (CONTROLLER)

//Inherits shared page display fields and functions
class AlbumController extends PageController
{
    private $albumModel;

    public function __construct()
    {
        parent::__construct();
        //Instantiate new album model
        $this->albumModel = new AlbumModel($this->getDate, $this->getRelease);
    }

    public function echoArtist()
    {
        echo $albumModel->artist;
    }

    public function isTracklisting()
    {
        if(!$this->strEmpty($albumModel->tracklisting)) return true;
    }

    public function echoTracklisting()
    {
        $songArray = explode(',', $albumModel->tracklisting);
        foreach($songArray as $song) {
            ...various logic...
        $html = '<li class="'.$class.'" '.$mp3Data.'>'.$song.'</li>';
        echo $html;
        }
    }

    ...etc...

PageController.php (CONTROLLER SUPER CLASS)

abstract class PageController
{
    protected $getDate;
    protected $getRelease;

    function __construct()
    {
        if(isset($_GET['date'])) $this->getDate = $_GET['date'];
        if(isset($_GET['release'])) $this->getRelease = $_GET['release'];
    }

    ...etc...

AlbumModel.php (MODEL)

class AlbumModel
{
    //Album data variables
    public $artist;
    public $album;
    public $genre;
    public $releaseDate;
    public $tracklisting;

    private $db;
    private $getDate;
    private $getRelease;

    public function __construct($getDate, $getRelease)
    {
        $this->getDate = $getDate;
        $this->getRelease = $getRelease;

        //Create new db object
        $this->db = new Database();

        //Get album array from db
        $albumArray = $this->getAlbumArrayFromDB();

        //Assign variables
        $this->artist = $albumArray['artist'];
        $this->album = $albumArray['album'];
        $this->genre = $albumArray['genre'];
        $this->releaseDate = $albumArray['date'];
        $this->tracklisting = $albumArray['tracklisting'];
    }

    private function getAlbumArrayFromDB()
    {
        //Query DB
        $sql = 'SELECT * FROM releases WHERE date = '.$this->getDate.' AND release_number = '.$this->getRelease;
        $qId = $this->db->query($sql);
        $albumArray = $this->db->fetch_array_assoc($qId);
        return $albumArray;
    }

    ...etc...

And my config.php defines constants and imports all my classes

config.php

//Paths
define('ABS_PATH', dirname(__FILE__));

//Database
define('DB_NAME', 'myname');
define('DB_USER', 'myuser');
define('DB_PASSWORD', 'mypass');
define('DB_HOST', 'myhost');

...etc...

//Import all classes
foreach(glob(dirname(__FILE__).'/classes/*.php') as $classPath) {
    require_once($classPath);
}

Am I understanding and implementing the MVC design pattern correctly?

I know strictly speaking it's not best practice to access instance variables directly (e.g. $albumModel->artist) but with dozens of additional variables it seems overkill/unnecessary to create getter functions for each one and then redefine them all again in the controller. I put these here because as I understand it the model defines the data, but I could just return an array to the controller and define these variables there?

Anything I'm missing or best practices I'm not adhering to?

Any advice greatly appreciated.

Many thanks

\$\endgroup\$

migrated from stackoverflow.com Jun 19 '13 at 21:06

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ Mostly you are missing MVC. Also adhering to few OOP practice and principles would be preferable. \$\endgroup\$ – tereško Jun 19 '13 at 20:07
  • \$\begingroup\$ Even if you don't want to use an existing framework it would be a good idea to look at some of them (FuelPHP, Laravel, CodeIgniter, Zend, etc.) to see how they do things and why. I would not include files (like header/footer/etc.) the way you are in your view. If you ever decide to have a "header2" you'll have to change every one of your views... \$\endgroup\$ – Benny Hill Jun 19 '13 at 20:08
  • \$\begingroup\$ In your example controller, you are generating HTML and just echoing it, then letting your view call that function. Ideally, a controller generates data (from the model) and returns it to the view and the view is responsible for the HTML-rendering work. In your code, they are more coupled than a controller and view should be. I agree with @BennyHill. Why do you not want to use an existing framework? Nobody says you have to use an existing one, but don't try to reinvent the wheel. \$\endgroup\$ – keaton_fu Jun 19 '13 at 20:10
  • \$\begingroup\$ Not using a framework for this is just wasted time trying to reinvent the wheel. Use Yii that is easy to learn, try to make some pages and then you will have knowledge about MVC. \$\endgroup\$ – Jorge Jun 19 '13 at 20:11
  • \$\begingroup\$ @Jorge .. only sad that Yii is complete nightmare that does not even implement MVC. It's a bad clone of Django. \$\endgroup\$ – tereško Jun 19 '13 at 20:11
5
\$\begingroup\$

albumView.php

Views are not glorified templates. They should be instances that acquire data from model layer and, based on that information, produce a response for the user.

Views do not:

  • initialize controllers
  • load configuration

What you currently have is not a view. It looks more like some variation on newbie's index.php file, that include everything ad outputs HTML.

AlbumController.php

The responsibility of controller in MVC design pattern is to alter the state of model layer and (sometimes) the current view instance, based on user's request.

Controllers do not:

  • create instances from model layer
  • route the request
  • generate HTML
  • extract data from model layer

AlbumModel.php

Model in MVC design pattern is a layer. Not a class or object. There are no "models". It is a singular layer that contains application, domain and storage logic.

P.S.: You obviously have no (or almost no) experience with OOP paradigm. You could start by watching/reading material that are listed here. This might provide you with a better grasp on the underlaying concepts.

\$\endgroup\$
  • \$\begingroup\$ @teresko You stated "Controllers do not extract data from model layer", surely they do if view requires data which needs to be processed? This would then be view->controller->model->controller->view ? ie view requests, controller grabs data from model (DB), controller checks it (whatever reason, makes sure it matches user input or perform some calculations based on view's request) then controller hands to view as an object (or array or variable) for view to output? (great answer, just trying to fine tune my understanding of it) \$\endgroup\$ – James Dec 10 '13 at 19:27
  • \$\begingroup\$ First of all, model layer is not synonymous with "DB". And you seem to assumes that "view" is a just a different name for template. It's a common form of misinformation, spread by Rails and Rails-like frameworks. I would suggest you to start with reading quick overview about the pattern (yes, it's wikipedia, deal with it). Pay special attention to the diagram on the side and "component interactions" section. When you re do with it, head over to this article. That should explain the basics. \$\endgroup\$ – tereško Dec 10 '13 at 20:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.