Leaks Can Become Floods

Leaks Can Become Floods

The Coder's Proverbs #3

The Coder's Proverbs is a series where I summarize some lessons and principles I've learned over my career by using a memorable and simple saying of wisdom.


So far we have talked about how we need to design having change in mind, and also established that the best changes are those that don't touch what already exists but add on top of it, and that we should design our applications so they can change that way. Today I would like to talk about being careful with our designs, particularly when we are designing abstractions.

What is an abstraction anyway?

Abstraction's only purpose is to help you simplify a problem or an API behind a stable contract. Some people see abstractions as something to be avoided like a plague and I understand where they are coming from. People who warn about hasty abstractions usually use the term abstraction meaning the extraction of a duplicated routine to keep the code DRY. This is not the definition of abstraction I have in mind.

The definition of abstraction I will use here relates more to contract definition than avoiding code duplication. In other words, this is an abstraction:

<?php

interface TemplateRenderer
{
    public function render(string $template, array $data = []): string;
}

TemplateRenderer is an interface, and as such, it is an abstraction over rendering a template. It does not specify how a template is rendered though (that's why is an abstraction), it just cares about the methods an implementation should have to be able to render a template.

Of course, abstractions are not only interfaces. Abstractions can be concrete classes too that assume an implicit "contract". But I'm more interested in interfaces this time.

Leaky Abstractions

The golden rule of designing interfaces as abstractions is that you should be able to swap implementations without needing to modify client code. So, if I have a TwigTemplateRenderer and a NativePhpTemplateRenderer implementing the TemplateRenderer above, I should be able to swap one for the other without changing anything in the code that uses the interface. If I need to change something, then I'm dealing with what is called a leaky abstraction.

We call leaky abstractions those abstractions that, intentionally or unintentionally, expose implementation details to client code. The fact that they expose implementation details forces you to change those details when you change the implementation.

There are two main ways in which this happens, at least in PHP, and that's through method arguments or exceptions. Let's suppose you have the following code using our above defined TemplateRenderer:

<?php

use Twig\Error\LoaderError;

class SendWelcomeEmail
{
    private TemplateRenderer $renderer;
    private Mailer $mailer;

    public function __construct(
        TemplateRenderer $renderer,
        Mailer $mailer
    )
    {
        $this->renderer = $renderer;
        $this->mailer = $mailer;
    }

    public function handle(UserRegistered $evt): void
    {
        try {
            $body = $this->renderer->render('emails/welcome.twig', [
                'username' => $evt->username,
                'name' => $evt->name,
            ]);
        } catch (LoaderException $e) {
            throw new LogicException('Template does not exist', previous: $e);
        }

        $email = (new Email())
            ->withBody($body)
            ->withSubject('Welcome')
            ->withFrom('My Awesome App <noreply@awesomeapp.com>')
            ->withTo(sprintf('%s <%s>', $evt->name, $evt->email))
        ;

        $this->mailer->send($email);
    }
}

So this code is a classical event listener for sending an email upon registration. You have a Mailer abstraction and a TemplateRenderer one. Can you spot the two problems with the TemplateRenderer one? I'll give you a clue. It has to do with the parts of the code that reference Twig.

The underlying problem here is that, although we are using an abstraction over rendering templates, the fact that we are using Twig as the internal engine leaks in this piece of code. It's pretty obvious to the code that this is using Twig. It has the Twig way of defining a template name (with slashes and with the .twig extension at the end) and it catches possible Twig exceptions. If I create another implementation of TemplateRenderer that does not use Twig but a native PHP renderer, I would need to change this code too.

This might sound like nitpicking, but it does have a big impact when your abstraction is widely used throughout the system. Although modern IDE tools would aid in changing all those references, not all developers have them. Plus, not all developers might know this is widely used and might just change the implementation anyway.

Fixing Leaks

You need to make sure that when you design an abstraction like a TemplateRenderer one, you think carefully about your use case. So let's fix this. First, it seems we need an error of our own to indicate that an irrecoverable failure happened when attempting to render a template. So let's define our own RenderError exception. And secondly, we need to make the way of identifying a template a bit more universal. Let's use dots and remove the extension.

<?php

use My\Namespace\Template\RenderError;

class SendWelcomeEmail
{
    private TemplateRenderer $renderer;
    private Mailer $mailer;

    public function __construct(
        TemplateRenderer $renderer,
        Mailer $mailer
    )
    {
        $this->renderer = $renderer;
        $this->mailer = $mailer;
    }

    public function handle(UserRegistered $evt): void
    {
        try {
            $body = $this->renderer->render('emails.welcome', [
                'username' => $evt->username,
                'name' => $evt->name,
            ]);
        } catch (RenderError $e) {
            throw new LogicException('Welcome email template could not be rendered', previous: $e);
        }

        $email = (new Email())
            ->withBody($body)
            ->withSubject('Welcome')
            ->withFrom('My Awesome App <noreply@awesomeapp.com>')
            ->withTo(sprintf('%s <%s>', $evt->name, $evt->email))
        ;

        $this->mailer->send($email);
    }
}

That's it. No trace of Twig anymore. Is up to the implementations now to catch the errors and transform them into RenderError and to modify the template name to change dots for slashes and add the file extension at the end.

So, when designing abstractions, take special care with the kind of arguments you are taking and ask yourself if they expose any of the internal details. And also, define exceptions that you are interested in communicating and make them part of the abstraction contract, so your consumers know what to catch and implementers know what to wrap.

Conclusions

I have done it again. This is the Liskov Substitution Principle applied to interfaces. Although this principle was first thought of in the context of subtyping with inheritance (the typical Square-Rectangle example) and modern OOP does not focus too much on inheritance anymore, it is still useful when considering interfaces, as they are a particular form of subtyping.

Abstractions that leak implementation details are one of those things few people pay attention to. Because it seems something so small and so trivial, is often dismissed or ignored. This might be the right response if the abstraction is not widely used. But if the abstraction turns out to be one of the centrepieces of your application, that leak can flood the rest of the application triggering a chain of change that could be hard to deal with.

So, keep an eye on your leaks, because they might become floods.

Did you find this article valuable?

Support The Chilean Nerd by becoming a sponsor. Any amount is appreciated!