How Terrible Code Gets Written By Perfectly Sane People

From Christian Maioli Mackeprang, the following:

Legacy code can be nasty, but I’ve been programming for 15 years and only a couple of times had I seen something like this. The authors had created their own framework, and it was a perfect storm of anti-patterns. …

And yet, it was not the code’s dismal quality that piqued my interest and led me to write this article. What I discovered after some months working there, was that the authors were actually an experienced group of senior developers with good technical skills. What could lead a team of competent developers to produce and actually deliver something like this?

I don’t necessarily buy all of these, but they’re worth thinking about:

  • Giving excessive importance to estimates: “Your developers might choose to over-promise instead, and then skip important work such as thinking about architectural problems, or how to automate tasks, in order to meet an unrealistic deadline.”

  • Giving no importance to project knowledge: “If you’re in a large project and there are modules for which you have no expert, no-one to ask about, that’s a big red flag.”

  • Focusing on poor metrics such as “issues closed” or “commits per day”: “When a measure becomes a target, it ceases to be a good measure. (Goodhart’s law)”

  • Assuming that good process fixes bad people: “[Fix your hiring.] Talent makes up for any other inefficiency in your team.”

  • Ignoring proven practices such as code reviews and unit testing: “Refraining from using tools such as modern IDEs, version control and code inspection will set you back tremendously.”

  • Hiring developers with no “people” skills: “It doesn’t matter that the other guy might be ten thousand miles away. One Skype call can turn a long coding marathon into a five-minute fix.”

Conclusion? “What you can’t assume is that just because you’ve signed up to apply Agile or some other tool, that nothing else matters and things will sort themselves out.”

Original MVC Resources from Reenskaug

This is from Trygve Reenskaug: MVC: Xerox PARC 1978-79. Some quotes:

The essential purpose of MVC is to bridge the gap between the human user’s mental model and the digital model that exists in the computer. The ideal MVC solution supports the user illusion of seeing and manipulating the domain information directly. The structure is useful if the user needs to see the same model element simultaneously in different contexts and/or from different viewpoints.

MVC was conceived as a general solution to the problem of users controlling a large and complex data set. The hardest part was to hit upon good names for the different architectural components. Model-View-Editor was the first set.

After long discussions, particularly with Adele Goldberg, we ended with the terms Model-View-Controller.

The MVC problem has more facets than I realized in 1979. I started working on a pattern language to disentangle the different aspects, that last draft was dated August 20, 2003. The plan was that it should be improved by a group of authors, not just the current single one. Unfortunately, the projct died at his point.

Slim and Action-Domain-Responder

I’ve had a warm place in my heart for Slim for a long time, and especially so since recognizing the Action-Domain-Responder pattern. In this post, I’ll show how to refactor the Slim tutorial application to ADR.

One nice thing about Slim (and most other HTTP user interface frameworks) is that they are already “action” oriented. That is, their routers do not presume a controller class with many action methods. Instead, they presume an action closure or a single-action invokable class. So the Action part of Action-Domain-Responder already exists for Slim. All that is needed is to pull extraneous bits out of the Actions, to more clearly separate their behaviors from Domain and the Responder behaviors.

I.

Let’s begin by extracting the Domain logic. In the original tutorial, the Actions use two data-source mappers directly, and embed some business logic as well. We can create a Service Layer class called TicketService and move those operations from the Actions into the Domain. Doing so gives us this class:

<?php
class TicketService
{
    protected $ticket_mapper;
    protected $component_mapper;

    public function __construct(
        TicketMapper $ticket_mapper,
        ComponentMapper $component_mapper
    ) {
        $this->ticket_mapper = $ticket_mapper;
        $this->component_mapper = $component_mapper;
    }

    public function getTickets()
    {
        return $this->ticket_mapper->getTickets();
    }

    public function getComponents()
    {
        return $this->component_mapper->getComponents();
    }

    public function getTicketById($ticket_id)
    {
        $ticket_id = (int) $ticket_id;
        return $this->ticket_mapper->getTicketById($ticket_id);
    }

    public function createTicket($data)
    {
        $component_id = (int) $data['component'];
        $component = $this->component_mapper->getComponentById($component_id);

        $ticket_data = [];
        $ticket_data['title'] = filter_var(
            $data['title'],
            FILTER_SANITIZE_STRING
        );
        $ticket_data['description'] = filter_var(
            $data['description'],
            FILTER_SANITIZE_STRING
        );
        $ticket_data['component'] = $component->getName();

        $ticket = new TicketEntity($ticket_data);
        $this->ticket_mapper->save($ticket);
        return $ticket;
    }
}
?>

We create a container object for it in index.php like so:

<?php
$container['ticket_service'] = function ($c) {
    return new TicketService(
        new TicketMapper($c['db']),
        new ComponentMapper($c['db'])
    );
};
?>

And now the Actions can use the TicketService instead of performing domain logic directly:

<?php
$app->get('/tickets', function (Request $request, Response $response) {
    $this->logger->addInfo("Ticket list");
    $tickets = $this->ticket_service->getTickets();
    $response = $this->view->render(
        $response,
        "tickets.phtml",
        ["tickets" => $tickets, "router" => $this->router]
    );
    return $response;
});

$app->get('/ticket/new', function (Request $request, Response $response) {
    $components = $this->ticket_service->getComponents();
    $response = $this->view->render(
        $response,
        "ticketadd.phtml",
        ["components" => $components]
    );
    return $response;
});

$app->post('/ticket/new', function (Request $request, Response $response) {
    $data = $request->getParsedBody();
    $this->ticket_service->createTicket($data);
    $response = $response->withRedirect("/tickets");
    return $response;
});

$app->get('/ticket/{id}', function (Request $request, Response $response, $args) {
    $ticket = $this->ticket_service->getTicketById($args['id']);
    $response = $this->view->render(
        $response,
        "ticketdetail.phtml",
        ["ticket" => $ticket]
    );
    return $response;
})->setName('ticket-detail');
?>

One benefit here is that we can now test the domain activities separately from the actions. We can begin to do something more like integration testing, even unit testing, instead of end-to-end system testing.

II.

In the case of the tutorial application, the presentation work is so straightforward as to not require a separate Responder for each action. A relaxed variation of a Responder layer is perfectly suitable in this simple case, one where each Action uses a different method on a common Responder.

Extracting the presentation work to a separate Responder, so that response-building is completely removed from the Action, looks like this:

<?php

use Psr\Http\Message\ResponseInterface as Response;
use Slim\Views\PhpRenderer;

class TicketResponder
{
    protected $view;

    public function __construct(PhpRenderer $view)
    {
        $this->view = $view;
    }

    public function index(Response $response, array $data)
    {
        return $this->view->render(
            $response,
            "tickets.phtml",
            $data
        );
    }

    public function detail(Response $response, array $data)
    {
        return $this->view->render(
            $response,
            "ticketdetail.phtml",
            $data
        );
    }

    public function add(Response $response, array $data)
    {
        return $this->view->render(
            $response,
            "ticketadd.phtml",
            $data
        );
    }

    public function create(Response $response)
    {
        return $response->withRedirect("/tickets");
    }
}
?>

We can then add the TicketResponder object to the container in index.php:

<?php
$container['ticket_responder'] = function ($c) {
    return new TicketResponder($c['view']);
};
?>

And finally we can refer to the Responder, instead of just the template system, in the Actions:

<?php
$app->get('/tickets', function (Request $request, Response $response) {
    $this->logger->addInfo("Ticket list");
    $tickets = $this->ticket_service->getTickets();
    return $this->ticket_responder->index(
        $response,
        ["tickets" => $tickets, "router" => $this->router]
    );
});

$app->get('/ticket/new', function (Request $request, Response $response) {
    $components = $this->ticket_service->getComponents();
    return $this->ticket_responder->add(
        $response,
        ["components" => $components]
    );
});

$app->post('/ticket/new', function (Request $request, Response $response) {
    $data = $request->getParsedBody();
    $this->ticket_service->createTicket($data);
    return $this->ticket_responder->create($response);
});

$app->get('/ticket/{id}', function (Request $request, Response $response, $args) {
    $ticket = $this->ticket_service->getTicketById($args['id']);
    return $this->ticket_responder->detail(
        $response,
        ["ticket" => $ticket]
    );
})->setName('ticket-detail');
?>

Now we can test the response-building work separately from the domain work.

Some notes:

Putting all the response-building in a single class with multiple methods, especially for simple cases like this tutorial, is fine to start with. For ADR, is not strictly necessary to have one Responder for each Action. What is necessary is to extract the response-building concerns out of the Action.

But as the presentation logic complexity increases (content-type negotiation? status headers? etc.), and as dependencies become different for each kind of response being built, you will want to have a Responder for each Action.

Alternatively, you might stick with a single Responder, but reduce its interface to a single method. In that case, you may find that using a Domain Payload (instead of “naked” domain results) has some significant benefits.

III.

At this point, the Slim tutorial application has been converted to ADR. We have separated the domain logic to a TicketService, and the presentation logic to a TicketResponder. And it’s easy to see how each Action does pretty much the same thing:

  • Marshals input and passes it into the Domain
  • Gets back a result from the Domain and passes it to the Responder
  • Invokes the Responder so it can build and return the Response

Now, for a simple case like this, using ADR (or even webbishy MVC) might seem like overkill. But simple cases become complex quickly, and this simple case shows how the ADR separation-of-concerns can be applied as a Slim-based application increases in complexity.

Why MVC doesn’t fit the web

[MVC is] a particular way to break up the responsibilities of parts of a graphical user interface application. One of the prototypical examples is a CAD application: models are the objects being drawn, in the abstract: models of mechanical parts, architectural elevations, whatever the subject of the particular application and use is. The “Views” are windows, rendering a particular view of that object. There might be several views of a three-dimensional part from different angles while the user is working. What’s left is the controller, which is a central place to collect actions the user is performing: key input, the mouse clicks, commands entered.

The responsibility goes something like “controller updates model, model signals that it’s been updated, view re-renders”.

This leaves the model relatively unencumbered by the design of whatever system it’s being displayed on, and lets the part of the software revolving around the concepts the model involves stay relatively pure in that domain. Measurements of parts in millimeters, not pixels; cylinders and cogs, rather than lines and z-buffers for display.

The View stays unidirectional: it gets the signal to update, it reads the state from the model and displays the updated view.

The controller even is pretty disciplined and takes input and makes it into definite commands and updates to the models.

Now if you’re wondering how this fits into a web server, you’re probably wondering the same thing I wondered for a long time. The pattern doesn’t fit.

Read the rest at Why MVC doesn’t fit the web.

The “Micro” Framework As “User Interface” Framework

(The following is more exploratory than prescriptive. My thoughts on this topic are incomplete and in-progress. Please try to treat it accordingly.)


tl;dr: “Micro” frameworks are better described as “user interface” frameworks; perhaps there should be corollary “infrastructure” frameworks; consider using two frameworks and/or two containers, one for the user interface and a separate one for the core application.

I.

When we talk about “full stack” frameworks, we mean something that incorporates tools for every part of a server-side application:

  • incoming-request handling through a front controller or router
  • dispatching routes to a page controller or action (typically a class constructed through a DI container)
  • database, email, logging, queuing, sessions, HTTP client, etc.; these are used in a service, model, controller, or action to support business logic
  • outgoing-response creation and sending, often combined with a template system

Examples in PHP include Cake, CodeIgniter, Fuel, Kohana, Laravel, Opulence, Symfony, Yii, Zend Framework, and too many others to count.

When we talk about “micro” frameworks, we mean something that concentrates primarily on the request-handling and response-building parts of a server-side application, and leaves everything else out:

  • incoming-request handling through a front controller or router
  • dispatching routes to a page controller or action (typically a closure that receives a service locator, though sometimes a class constructed through a DI container)
  • outgoing-response creation and sending, often combined with a template system

Slim is the canonical PHP example here; others include Equip, Lumen, Silex, Radar, Zend Expressive, and (again) too many others to count.

II.

I have asserted elsewhere that, in an over-the-network HTTP-oriented server-side application, the View (of Model-View-Controller) is not the template. The View is the entire HTTP response: not just the HTML or the body, but the status line, the headers, all cookies, and so on. That is, the full HTTP response is the presentation; it is the output of the server-side application.

If the HTTP response is the output, then what is the user input into the server-side application? It is not the form being submitted by the user, per se. It is not a key-press or button-click or mouse-movement. The input is the entire HTTP request, as sent by the client to the server.

That means the user interface for a server-side application is not “the HTML being viewed on the screen.” The user interface is the HTTP request and response.

Now, for the big jump:

If the user interface is the request (as input), and the response (as output), that means micro-frameworks are not so much “micro” frameworks, as they are “user interface” frameworks.

Consider that for a moment before continuing.

III.

If it is true that “micro” frameworks are more accurately described as “user interface” frameworks, it opens some interesting avenues of thought.

For one: it means that full-stack frameworks combine user interface concerns with other concerns (notably but not exclusively infrastructure concerns).

For another: perhaps in addition to user interface (“micro”) frameworks, it would be useful to have something like a separate “infrastructure” framework. It might not even be a framework proper; it might only be a container for shared services that have no relation to any user interface concerns. (I can imagine extracting such a toolkit collection from any of the full-stack frameworks, to be offered on its own.)

IV.

Speaking of containers:

It is often the case, even with user interface (“micro”) frameworks, that developers will store their infrastructure and services objects in the framework-provided container. Or, that they will build “providers” or “configuration” for the framework-provided container, so that the container can create and retain the infrastructure and services objects.

But if it is true that the “micro” framework is a “user interface” system, why would we manage infrastructure and domain objects through a user interface container? (This a variation on “Why is your application/domain/business logic in your user interface controller?”)

Perhaps it would be better for the infrastructure and/or domain objects to be managed through a container of their own. Then that completely separate container can be injected into any user interface container. The benefit here is that the infrastructure and/or domain objects are now fully separated from the user interface layer, and can be used with any user interface framework, even one that uses a completely different container for its own user interface purposes. (This is a variation on “composition to reduce coupling.”)

V.

To conclude:

It might be worth considering the use of two frameworks in server-side applications, each with its own container: one for the user interface concerns, and a separate one for infrastructure/domain concerns. (The latter might be composed into any user interface framework, whether one for the web or one for the command line.)

And, given that lots of application work starts mostly as infrastructure coordination, with little domain logic, having a second framework separated from the user interface makes it easier to put a boundary around the infrastructure-based application code, separating it from the user interface code. The work on each can proceed in parallel; the user interface coders can mock the results from the application layer, and build everything out on their own path, while the application coders don’t need to worry too much about presentation concerns (whether HTTP or CLI based).


Unlike some other people, I am not my code, or my books, or my blog posts. Given the tentative nature of the above essay, review and criticism (especially substantial criticism) are vital. It won’t hurt me in the least bit; indeed, I encourage it.

Thanks for reading.

UPDATE: The Reddit comments regarding this post are … illustrative.