Model-View-Controller (MVC) Mistakes

If you're among the MVC crowd, you're probably chastising my examples already for their poor design. I admit, they offer no separation of the data, code (logic and glue), or template layers at all. Let's see how we might abstract this same example in Sitellite, implementing a proper Model-View-Controller design pattern.

First, we need to abstract the SQL into its own library. We can do this simply by creating a class which contains the SQL code as a method, like this:

<?php

class NewsStory {
    function headlines () {
        return db_fetch_array (
            'select id, title
            from sitellite_news
            order by date desc
            limit 10'
        );
    }
}

?>

Or we can use a better method, which is built into the Sitellite Database API in a package called Generic. The following code was taken from the inc/app/news/lib/Story.php file, so you don't need to create it yourself.

<?php

loader_import ('saf.Database.Generic');

class NewsStory extends Generic {
    function NewsStory () {
        parent::Generic ('sitellite_news', 'id');
        $this->usePermissions = true;
    }
}

?>

As you can see, this code is very short, and doesn't appear to do much other than define a class. However, that's exactly the benefit of Generic. In fact, we can now rewrite our inc/app/example/boxes/headlines/index.php file as follows:

<?php

loader_import ('news.Story');

$story = new NewsStory;

$story->limit (10);
$story->orderBy ('date desc');

$results = $story->find (array ());
if (! $results) {
    $results = array ();
}

echo template_simple (
    '<ul>{loop obj}
        <li>
            <a href="{site/prefix}/index/tutorials-story-action/story.{loop/id}">
                {loop/title}
            </a>
        </li>
    {end loop}</ul>',
    $results
);

?>

The only problem is we've just increased the amount of code, and decreased its readability. So instead of putting all this implementation detail into the index.php file itself, we could instead add that headlines() method from before to our NewsStory class, like this:

<?php

loader_import ('saf.Database.Generic');

class NewsStory extends Generic {
    function NewsStory () {
        parent::Generic ('sitellite_news', 'id');
        $this->usePermissions = true;
    }

    function headlines ($limit = 10) {
        $this->clear ();
        $this->limit ($limit);
        $this->orderBy ('date desc');

        $results = $this->find (array ());
        if (! $results) {
            return array ();
        }
        return $results;
    }
}

?>

Now our index.php file becomes simply:

<?php

loader_import ('news.Story');

$news = new NewsStory;

echo template_simple (
    '<ul>{loop obj}
        <li>
            <a href="{site/prefix}/index/tutorials-story-action/story.{loop/id}">
                {loop/title}
            </a>
        </li>
    {end loop}</ul>',
    $news->headlines ()
);

?>

Much closer to what we had before, and much more readable. For those not in the know about the Model-View-Controller (MVC) design pattern, I'll now extoll some of the virtues of this method over the first approach we took.

• In this new method, we've separated the logic of the application from the code that glues it together with the current visitor request (the glue code).
• We've also eliminated completely the hard-coded SQL query, thereby increasing the portability of our code from one database server to another.
• If we were to add new logic to our news story application, we now have a single location for all the logic to go, as opposed to spreading it out over a dozen different scripts. This makes an application much less error-prone, and also more maintainable, because a single logical change doesn't have to be repeated in a handful of different places.
• Another thing I didn't mention yet, but that you probably noticed, about our NewsStory class is the line "$this->usePermissions = true;". This line is particularly handy because our old code failed to consider the permissions of the user when retrieving the headlines, but our new code did so automatically for us.
• We've also made the limit in our headlines() function a parameter, so you can change it by specifying it in the index.php file if you like. For fun, try to figure out how you might make that a configurable option based on the examples here, and the existing sidebars provided with Sitellite (to see how to pass optional values to a box).

Although it's not exactly related to the Database API, I should do one last thing to complete the MVC transformation of our code. Namely, our template should be separate from the glue code too. Separating these out leaves us with one new file, and a small change to the index.php file. In inc/app/example/html/headlines.spt we have:

<ul>{loop obj}
    <li>
        <a href="{site/prefix}/index/tutorials-story-action/story.{loop/id}">
            {loop/title}
        </a>
    </li>
{end loop}</ul>

And in index.php we now have:

<?php

loader_import ('news.Story');

$news = new NewsStory;

echo template_simple (
    'headlines.spt',
    $news->headlines ()
);

?>

If you ask me, I'd say we've ended up with clean, clear, transparent, and secure application code, using the Sitellite Database API and a few standard programming concepts. What more could you ask for?

Page 1: Rationale
Page 2: A Simple Example
Page 3: Model-View-Controller (MVC) Mistakes

All Tutorials