Practical PHP Refactoring: Separate Domain from Presentation

PHP is unfortunately famous for spaghetti code, but instances of tangle logic is a product of the programmer and not of the language.

One of the anti-patterns we have learned to avoid is to tangle domain logic and presentation logic. Domain logic consists of data and behavior models such as User, Group and ForumPost classes and the database for their persistence; presentation logic regards producing an output, in the form of HTML, JSON or any other format. Moreover, analyzing an HTTP request and its headers is also part of the upper layer of an application, which should conflate all presentation logic.

The Separate Domain from Presentation refactoring tries to take you away from the SQL-and-HTML-in-the-same-script approach to target an architectural pattern like layering or MVC2.

But I'm using a framework, it "separates concerns" for me

Even when using an MVC (sometimes named MVC2 in its web version) framework, it's not necessarily the case that domain and presentation logic are actually separated.
Frameworks usually force you to write controllers and views, and this move forces to separate response templating (producing HTML) from executing a request (modifying or retrieving the state of the application, usually with a session or database as back end).

However, frameworks do not address the separation of models from controller logic dictated by MVC. The reason is probably the choices between possible models and for their persistence are so different that the framework gives you base classes for controllers and views, but not for models. The result is commonly a giant controller class, containing all the logic of the application: this solution conflates HTTP and application-level concerns like parameters and authentication with domain-specific logic like how the most recent posts are calculated or which information the user should provide in order to register.

So why separation?

A first reason is greater cohesion of your classes and namespaces: constructs that change together should stay together in a class or a folder. In a modern apporach, when you change how to select the post for a query, you change only a Posts class; when you change the markup, you change only the template.
That doesn't mean there aren't evolutions which are even better at separating concerns vertically (across entities and fields) and not only horizontally (across layers).

But at least separating the markup from everything else if the first step.

The second reason is testability. If you can isolate the pieces of your design, you can test them by themselves; unit tests are both easier to setup due to the reduced scope, and more deterministic (hence automatable) since you lack dependence on global variables like a date or random numbers. Imagine setting your clock to the right time test the code is correct in some situations...

The result of separation of concerns mean you can:

• test a controller or an action without Apache but only with PHPUnit.
• test the persistence of a model with a Fake database, by populating SQLite tables.
• test a template (if you want to) without having to populate tables.

Steps

1. Identify the purpose of the tangled page and create a domain class for it. Different operations may be modelled as different methods on the same object (e.g. a Repository satisfying every query regarding users) if you have already many domain classes.
2. Move the logic away from the current place and into the domain class.
3. Extract a template file or a View Helper to generate HTML (or JSON or what your output consists of).
4. After the second movement, the logic inside the original page or action should be centered around the HTTP request and in wiring together the other objects. This is a rudimental MVC pattern for the web.

Example

We operate on a .php script, for simplicity; the goal is to separate it into a model, a view and a controller. The same example could be applied to a controller action built in Symfony or Zend Framework.

This script prints the first not read post in a thread marked as sticky by the administrators; it returns it as an HTML fragment that can be included by clients.
There is optional parameter, a last_visit date: in case it is present, it means no posts before this date should be considered. In case this parameter is absent, it means there are no informations about the current user and so a recent post should be selected.

This MySQL contains the data about posts:

mysql> SELECT * FROM posts;
+----+-----------+---------+----------------+------------+
| id | id_thread | author  | text           | date       |
+----+-----------+---------+----------------+------------+
|  1 |        23 | giorgio | My new post... | 2012-02-13 |
|  2 |        23 | giorgio | My old post... | 2012-01-01 |
+----+-----------+---------+----------------+------------+
2 rows in set (0.00 sec)

<?php
$dsn = 'mysql:host=localhost;dbname=practical_php_refactoring';
$username = 'root';
$password = '';
$dbh = new PDO($dsn, $username, $password);
$stmt = $dbh->prepare("SELECT * FROM posts WHERE id_thread = :id_thread AND date >= :date ORDER BY date");
$stmt->bindValue(':id_thread', (int) $_GET['id_thread']);
if (!isset($_GET['last_visit'])) {
    $_GET['last_visit'] = date('Y-m-d');
}
$stmt->bindValue(':date', $_GET['last_visit']);
$stmt->execute();
$post = $stmt->fetch();
?>
<div class="post">
    <div class="author"><?php echo $post['author']; ?></div>
    <div class="date"><?php echo $post['date']; ?></div>
    <div class="text"><?php echo $post['text']; ?></div>
</div>

We extract a model for the posts, but since each of them has very little logic by itself (a Post does not have states in this model, or validation during creation/update) it's mostly about encapsulating the queries (a Repository pattern).

<?php
class Posts
{
    public function __construct(PDO $connection)
    {
        $this->connection = $connection;
    }

    /**
     * @param int $threadId
     * @param string $lastVisit Y-m-d format
     * @return array    fields for the selected post
     */
    public function lastPost($threadId, $lastVisit)
    {
        $stmt = $this->connection->prepare("SELECT * FROM posts WHERE id_thread = :id_thread AND date >= :date ORDER BY date");
        $stmt->bindValue(':id_thread', (int) $_GET['id_thread']);
        $stmt->bindValue(':date', $lastVisit);
        $stmt->execute();
        return $stmt->fetch();
    }
}
$dsn = 'mysql:host=localhost;dbname=practical_php_refactoring';
$username = 'root';
$password = '';
$posts = new Posts(new PDO($dsn, $username, $password));
if (!isset($_GET['last_visit'])) {
    $_GET['last_visit'] = date('Y-m-d');
}
$post = $posts->lastPost($_GET['id_thread'], $_GET['last_visit']);
?>
<div class="post">
    <div class="author"><?php echo $post['author']; ?></div>
    <div class="date"><?php echo $post['date']; ?></div>
    <div class="text"><?php echo $post['text']; ?></div>
</div>

In real life you would probably compose an EntityManager or another Facade for the ORM, instead of directly accessing PDO. Note that the classes shown here are kept in the original file for the sake of a self-contained example, but they should be moved into their own sourcefile and autoloaded.

Now we extract the controller, but actually it's just a single action of a possible controller. We'll call the class Action to reflect this fact.

<?php
class Posts
{
    public function __construct(PDO $connection)
    {
        $this->connection = $connection;
    }

    /**
     * @param int $threadId
     * @param string $lastVisit Y-m-d format
     * @return array    fields for the selected post
     */
    public function lastPost($threadId, $lastVisit)
    {
        $stmt = $this->connection->prepare("SELECT * FROM posts WHERE id_thread = :id_thread AND date >= :date ORDER BY date");
        $stmt->bindValue(':id_thread', (int) $_GET['id_thread']);
        $stmt->bindValue(':date', $lastVisit);
        $stmt->execute();
        return $stmt->fetch();
    }
}
class LastPostAction
{
    public function __construct(Posts $posts, $template)
    {
        $this->posts = $posts;
        $this->template = $template;
    }

    public function execute(array $getParameters)
    {
        if (!isset($getParameters['last_visit'])) {
            $getParameters['last_visit'] = date('Y-m-d');
        }
        $post = $this->posts->lastPost($getParameters['id_thread'],
                                       $getParameters['last_visit']);
        require $this->template;
    }
}
$dsn = 'mysql:host=localhost;dbname=practical_php_refactoring';
$username = 'root';
$password = '';
$action = new LastPostAction(
                new Posts(new PDO($dsn, $username, $password)),
                'last_post.php'
          );
$action->execute($_GET);
?>

Together with the controller, we extracted also the template, finally separating presentation from eveyrthing else. Since PHP is already a templating language, the result is quite clean.

<div class="post">
    <div class="author"><?php echo $post['author']; ?></div>
    <div class="date"><?php echo $post['date']; ?></div>
    <div class="text"><?php echo $post['text']; ?></div>
</div>