SOLID: Open Closed Principle

  • 24/03/2016
  • 10 minuten leestijd

SOLID: Open Closed Principle

The third part of the series on the SOLID principles, based on the technight on the same subject.

A bit of history

The Open Closed Principle (OCP), is one of the two oldest of the SOLID principles, hailing from 1988[1]. As such I need to tell you a little bit about its history. It is, after all, almost 20 years old, which is an eternity in computer science terms. (I consider the history of computer science to start in 1937, when Turing first proposed the possibility of a stored-program machine[2]. This makes the full period not even 80 years long, of which 20 is more than 25%. So, I could compare OCP's place in computer science history to the place of the first printing presses in the history of data transfer. Which makes it very old. Always nice to realize that what we are doing is still only making its very first baby steps).

The OCP came to be in a time where most new code was dependent on pre-compiled libraries of code that was written by other programmers. Being already compiled, these libraries could not be altered anymore, for which Bertrand used the term "closed". In contrast, code to which you could add new fields and method was considered "open". The problem was that ideally, code should be both: you should be able to use these pre-compiled libraries, but also change them. Which led Bertrand to coin the Open/Closed principle as such:

software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification [1]

Essentially he solved the issue by stating that code must be closed for modification, which meant it could be compiled, but open for extension: being extensible allows desired modifications —that can not be placed in the parent class— to be placed in the child class.

Nowadays, most languages still have this issue. Some languages, such as PHP, have some libraries that could be changed (e.g.: everything that is included through composer), but I'd advise against that, hopefully for obvious reasons. And if it is not immediately obvious, this post will try to convince you to consider it closed anyways.

Open

At first sight, the OCP may appear to be a language feature, rather than a design principle. After all, as long as the code is compiled, it will be closed for modification, and if the language allows for the extends keyword (or something similar), it is open for extension, thus upholding the OCP in language features alone.

However, this is not the full story. Examine for example the following code:

<?php

final class BottleOpener
{
    /**
     * Opens a bottle.
     *
     * @param Bottle $bottle
     */
    public function openBeer(Bottle $bottle)
    {
        switch ($bottle->lidType)
        {
            case "CrownCap":
                return $this->openWithTorque($bottle);
            case "SwingTop":
                return $this->openWithThumbs($bottle);
        }
        return $this->breakBottle($bottle);
    }

    /**
     * Opens a bottle by applying torque to the crown cap and flipping it off
     * the bottle.
     *
     * @param Bottle $bottle
     */
    public function openWithTorque(Bottle $bottle) { /* ... */ }

    /**
     * Opens a bottle by pressing the thumb against the release mechanism, thus
     * removing the cap from the bottle
     *
     * @param Bottle $bottle
     */
    final public function openWithThumbs(Bottle $bottle) { /* ... */ }

    /**
     * Opens a bottle by carefully smashing it against an object until the
     * content spills out.
     *
     * @param Bottle $bottle
     */
    private function breakBottle(Bottle $bottle) { /* ... */ }
}

This code is not open for extension for multiple reasons. The first reason should be immediately obvious: the class is marked final. Which explicitly states that it can not be extended. You'd think this means the keyword itself breaks OCP, and I'd agree, although this is debatable (see e.g. [3]), and particularly the parts about the prerequisites of using the final keyword). All in all, I'd advice against using this keyword altogether; it makes your code harder to extend, without any real advantages.

Of course, the same goes for the final keyword on the openWithThumbs method, although declaring a method final is somewhat different from declaring classes as such. It means the method can't be overwritten, which makes the behaviour closed for extension, but not the class itself. This is therefore slightly less breaking of OCP, but I'd still avoid it in public methods. For non-public methods however, various scenarios exist wherein using the final keyword is proper and good (e.g: anything that has to do with sending password information to other services).

Related to final methods is making methods private instead of protected. It means extending classes can't use the method, making the code slightly more closed for extension. If the class is properly written —with private methods that are nothing more than small, reusable pieces of code, that extending classes should never use— this is in fact not an issue. However, I've often run into libraries that do not do this properly, such as ORM's that make the query execution method itself private. If we wish to extend those ORM's with additional functionality, we do not have access to the execution method, which prevents us from extending it in a proper way. So I'd consider theprivate keyword as being suspect as well.

But the best example of breaking the OCP in the BottleOpener would be the content of the openBeer method. It uses a fairly limited switch statement to determine the way the beer is opened. If I'd wish to extend the BottleOpener to be able to open a beer bottle that is closed using a cork, I would have to rewrite of the BottleOpener altogether. After all, extending the class does not allow me to add pieces to the switch statement. Again, you could code around this issue (e.g.: overwriting the openBeer method and calling the parent implementation if the lidType is not a cork), but in extremis this is no different from simply writing a new class and not using the library code at all.

For arguments sake, the following example shows the same behaviour as the previous version, but with all issues of closedness removed:

<?php

class BottleOpener
{
    /** @var \Closure[] */
    private $openers;

    /**
     * BottleOpener constructor.
     */
    public function __construct()
    {
        $this->registerOpener('CrownCap', function (Bottle $bottle) {
            return $this->openWithTorque($bottle);
        });
        $this->registerOpener('SwingTop', function (Bottle $bottle) {
            return $this->openWithThumbs($bottle);
        });
    }

    /**
     * Tells the bottle opener about a new $lidType and $opener method to use,
     * thus preventing the opener from breaking bottles of this type
     *
     * @param string $lidType
     * @param Closure $opener
     */
    public function registerOpener(string $lidType, \Closure $opener)
    {
        $this->openers[$lidType] = $opener;
    }

    /**
     * Opens a bottle using the registered openener.
     *
     * @param Bottle $bottle
     */
    public function openBeer(Bottle $bottle)
    {
        if (array_key_exists($bottle->lidType, $this->openers)) {
            return $this->openers[$bottle->lidType]($bottle);
        }
        return $this->breakBottle($bottle);
    }

    /**
     * Opens a bottle by applying torque to the crown cap and flipping it off
     * the bottle.
     *
     * @param Bottle $bottle
     */
    public function openWithTorque(Bottle $bottle) { /* ... */ }

    /**
     * Opens a bottle by pressing the thumb against the release mechanism, thus
     * removing the cap from the bottle
     *
     * @param Bottle $bottle
     */
    protected function openWithThumbs(Bottle $bottle) { /* ... */ }

    /**
     * Opens a bottle by carefully smashing it against an object until the
     * content spills out.
     *
     * @param Bottle $bottle
     */
    protected function breakBottle(Bottle $bottle) { /* ... */ }
}

The final keywords have been removed, the private breakBottle method has been changed to protected and the switch statement has been replaced by a look up in an array, to which methods can be added at will. Note that the $openers are declared private, since they should only be accessed through the registerOpener and openBeer methods: private is not all bad...

Closed

In the previous section, we discussed how to keep the BeerOpener open for extension, but keeping it closed for modification is just as important. Let's show that through an example:

Some not-too-smart developer comes along and is asked to implement the baptizing of boats. To do that, he determines that part of the code must be able to break a champagne bottle. He implements this as follows:

<?php

class ChampagneBottle extends Bottle
{
    /** @var string */
    public $lidType = "Cork"; // Remember: this is not implemented in switch
}

class Boat
{
    /**
     * Baptizes the boat using various actions
     *
     * @param BottleOpener $opener
     * @param ChampagneBottle $bottle
     */
    public function baptize(BottleOpener $opener, ChampagneBottle $bottle)
    {
        // ...
        $opener->openBeer($bottle); // Easiest way to break a bottle I could find!
        // ...
    }
}

It's a dirty solution, but I have to agree that it works. However, the developer of the bottleOpener is now required to never add handling of bottles with corks to his openBeer method, since if he does, he inadvertently introduces a bug in the baptize method! The worst part about this is that the developer has no real way of knowing that he should never add this option! After all, how could he know there are developers that depend on the lack of functionality for opening bottles with corks?

The answer is quite simple: the BottleOpener must be closed for modification. So, regardless of whether code exists that uses that particular behaviour, he should never edit this "library" code.

Pitfalls

I can already hear you think: "Do you mean the OCP says I can't change my code anymore once it has reached production?". In a way, yes, I do. But it is of course not as clear cut as that, as that would make fixing bugs a lot harder, instead of easier. So I'd say that determining whether code is "library" code or not is quite important: if you know where the code is being used —or the full application that uses the code has proper unit test code coverage— you can change it after all, because the code is not "library" code.

Therefore, the one pitfall of the OCP would be considering code to be library code when in fact it is not, or vice versa. Please make sure to think twice when making this determination.

Conclusion

The OCP is a principle that tells us how to act around library code. It should be closed for modification because we do not know what code actually uses the behaviour we intend to change, but in order to be able to add that behaviour anyways, the code must be open for extension.

Holding to these implicit agreements gives us both the robustness of knowing the library code will always have that particular behaviour, but it still allows us the flexibility of changing it.

Some situations can arise in your code that probably indicate you're likely breaking the OCP:

  • Using the final keyword
  • Switch statements based on non-constant string values
  • Being forced to change library code directly
  • Not being able to easily mock a class

Finally, it has to be noted that later on, we will handle the Dependency Inversion Principle, which, if used throughout your application, solves most of these issues. After all, interfaces are by definition open for extension and closed for modification.

By the way, in the post about the Single Responsibility Principle, I argued that using the extends keyword is often the wrong way to go. Extending closed code with new behaviour is the exception to the rule here. In my opinion it is the reason the keyword exists.

References