The refactoring breakthrough on a CoffeeMachine

Today I will write about a concept I came to know from Domain-Driven Design, Eric Evans's book centered on the Domain Model pattern. DDD isn't just Entities, Value Objects, Repositories, Factories and Services - is an entire approach for the development of an application where the Domain Model is a top priority. This methodology adapts well to very complex domains, but we can learn valuable lessons that applies everywhere.

Refactoring

Usually, we spread our refactoring effort working on it all the time; not on all the available time of course, but never skipping a refactoring step, which is the third part of the Test-Driven Development mini-cycle. After you have added a failing test (red) and making it pass with some lines of additional code (green), you have to review your solution to clean it (refactor).

Why we take this approach? Because when the code is fresh it's easier to refactor, while when the non encapsulated part of it scatters in the code base it becomes more rigid (that's why we bother with encapsulation/information hiding.) For example, the easiest moment for changing an Api is when no one uses it, and only the class that exposes it and its tests know any details. While TDDing, you write the test for a class ahead of the Api, so you can change the Api even before it exists.

A critique that can be moved to continuous refactoring and refinement of the model is that most of the techniques we apply to the code do not add enough value to cover their costs.

I believe this is never true. I'm not advocating that we should refactor every piece of code, even when it is not expected to ever change. For example, the consistency of private variable names in different classes is probably not of much value to the potential of the classes themselves, but only to the programmers. But there is still value even in small refactorings.

The Boy Scout Rule proposed by Uncle Bob is a good rule of thumb here: when you come in contact with some code, change it (while remaining protected by regression tests) to make it only a little better than it was, commit a tiny refactoring along with your main changes. A variable name, a private method signature at the time the code will get cleaner.

Breakthrough

The concept of refactoring breakthrough means that the value of refactoring is not a linear function of the effort and time applied to it. So the small refactorings we do all the time are not a waste of time, they're necessary steps.

For starters, they lead us to overlearning the part of the domain and domain model we are working on. But there's more to small refactorings than that.

On x-axys effort spent, on the y-axis value gained by refactoring towards a deeper insight. Tiny, small refactorings open the way for earth-shattering ones. We are not talking about major refactoring nor rewritings from scratch: giant leaps are difficult to take. But one step at the time, improving the structure of the code can expose a pattern we would want to make explicit, and allow us to introduce new objects and classes without a real major refactoring (which for example changes 42 classes in a single commit, and we can't commit earlier because it would break the build.)

The example of Evans is about managing loans which involve with multiple lenders (complex systems, you know). After several code tunings and refactorings, he introduces the Share interface with several implementations, a new class that emerges from the Domain Model naturally thanks to the previous tweakings to the code.

DDD is for complex systems, but also web applications are complex systems nowadays. We're not stll shuffling the data from a form to a database to a page. We use ORMs, libraries and frameworks. We interact with web services and streams. We read and produce files. We respond to requests from different kind of clients - browsers, mobile browsers and other applications, and all of them can talk in plain old HTTP paradigm or in AJAX. A refactoring breakthrough can happen everywhere.

Examples

Showing some code is crucial here or I would never explain myself. This example show how continuous refactoring of validation rules leaded to introduce a parameter object.

Here I present only key steps, but all the code is on github. I committed very often to show the steps I took while TDDing and refactoring.

So we have a CoffeeMachine, and we want to load it with supplies. The problem is - the coffee supplies are single-packaged, while the hot chocolate ones come in a package of 10, and we can only insert that in the CoffeeMachine.

The test after coding a bit is this:

<?php
require_once 'CoffeeMachine.php';

class CoffeeMachineTest extends PHPUnit_Framework_TestCase
{
    private $machine;

    public function setUp()
    {
        $this->machine = new CoffeeMachine(array('Coffee', 'Chocolate'));
    }

    public function testMachineIsLoadedWithZeroSuppliesAtCreation()
    {
        $this->assertSuppliesAre(0, 'Coffee');
        $this->assertSuppliesAre(0, 'Chocolate');
    }

    public function testMachineCanBeLoadedWithCoffeeSupplies()
    {
        $this->machine->loadSupplies('Coffee', 5);

        $this->assertSuppliesAre(5, 'Coffee');
        $this->assertSuppliesAre(0, 'Chocolate');
    }

    public function testMachineCanBeLoadedWithChocolateSupplies()
    {
        $this->machine->loadSupplies('Chocolate');

        $this->assertSuppliesAre(0, 'Coffee');
        $this->assertSuppliesAre(10, 'Chocolate');
    }

    private function assertSuppliesAre($number, $beverageName)
    {
        $this->assertEquals($number, $this->machine->getSupplies($beverageName));
    }
}

while the production code is a bit ugly:

<?php
class CoffeeMachine
{
    /**
     * @array
     */
    private $supplies;

    public function __construct(array $beverages)
    {
        $this->supplies = array();
        foreach ($beverages as $name) {
            $this->supplies[$name] = 0;
        }
    }

    public function loadSupplies($beverageName, $quantity = 10)
    {
        $this->supplies[$beverageName] += $quantity;
    }

    public function getSupplies($beverageName)
    {
        return $this->supplies[$beverageName];
    }
}

To strengthen things up a bit, we add the requirement that you cannot call $machine->loadSupplies('Coffee') and use the 10 default parameter in the test (it's intended for Chocolate.) The production code responds with:

<?php
class CoffeeMachine
{
    /**
     * @array
     */
    private $supplies;

    public function __construct(array $beverages)
    {
        $this->supplies = array();
        foreach ($beverages as $name) {
            $this->supplies[$name] = 0;
        }
    }

    public function loadSupplies($beverageName, $quantity = null)
    {
        if ($quantity === null) {
            if ($beverageName == "Chocolate") {
                $quantity = 10;
            } else {
                throw new InvalidArgumentException("Only Chocolate has a predefined quantity of supplies to load.");
            }
        }
        $this->supplies[$beverageName] += $quantity;
    }

    public function getSupplies($beverageName)
    {
        return $this->supplies[$beverageName];
    }
}

Not much better. We factor out a method to deal with the $quantity possible values:

<?php
class CoffeeMachine
{
    /**
     * @array
     */
    private $supplies;

    public function __construct(array $beverages)
    {
        $this->supplies = array();
        foreach ($beverages as $name) {
            $this->supplies[$name] = 0;
        }
    }

    public function loadSupplies($beverageName, $optionalQuantity = null)
    {
        $quantity =  $this->getSuppliesQuantity($beverageName, $optionalQuantity);

        $this->supplies[$beverageName] += $quantity;
    }

    private function getSuppliesQuantity($beverageName, $quantity)
    {
        if ($quantity === null) {
            if ($beverageName == "Chocolate") {
                return 10;
            } else {
                throw new InvalidArgumentException("Only Chocolate has a predefined quantity of supplies to load.");
            }
        }
        return $quantity;
    }

    public function getSupplies($beverageName)
    {
        return $this->supplies[$beverageName];
    }
}

Now we shake up things, and add that Coffee supplies come in packages where items are multiple of 5: 5, 10 and 15 are correct supplies, but 11 or 4 aren't.

<?php
class CoffeeMachine
{
    /**
     * @array
     */
    private $supplies;

    public function __construct(array $beverages)
    {
        $this->supplies = array();
        foreach ($beverages as $name) {
            $this->supplies[$name] = 0;
        }
    }

    public function loadSupplies($beverageName, $optionalQuantity = null)
    {
        $quantity =  $this->getSuppliesQuantity($beverageName, $optionalQuantity);

        $this->supplies[$beverageName] += $quantity;
    }

    private function getSuppliesQuantity($beverageName, $quantity)
    {
        if ($quantity === null) {
            if ($beverageName == "Chocolate") {
                return 10;
            } else {
                throw new InvalidArgumentException("Only Chocolate has a predefined quantity of supplies to load.");
            }
        }
        if ($beverageName == "Coffee" and $quantity % 5 != 0) {
            throw new InvalidArgumentException("Coffee supplies must come in multiples of 5.");
        }
        return $quantity;
    }

    public function getSupplies($beverageName)
    {
        return $this->supplies[$beverageName];
    }
}

The getSuppliesQuantity() method is becoming long. We refactor it to differentiate between Chocolate and Coffee:

<?php
class CoffeeMachine
{
    /**
     * @array
     */
    private $supplies;

    public function __construct(array $beverages)
    {
        $this->supplies = array();
        foreach ($beverages as $name) {
            $this->supplies[$name] = 0;
        }
    }

    public function loadSupplies($beverageName, $optionalQuantity = null)
    {
        $quantity =  $this->getSuppliesQuantity($beverageName, $optionalQuantity);
        $this->supplies[$beverageName] += $quantity;
    }

    private function getSuppliesQuantity($beverageName, $quantity)
    {
        if ($beverageName == "Chocolate") {
            if ($quantity === null) {
                return 10;
            }
        }

        if ($beverageName == "Coffee") {
            if ($quantity === null) {
                throw new InvalidArgumentException("Coffee supplies number must be defined.");
            }
            if ($quantity % 5 != 0) {
                throw new InvalidArgumentException("Coffee supplies must come in multiples of 5.");
            }
        }
        return $quantity;
    }

    public function getSupplies($beverageName)
    {
        return $this->supplies[$beverageName];
    }
}

Now Coffee and Chocolate supplies are well separated. Here the breakthrough happens: we introduce two objects that handle the validation of $quantity:

<?php
class CoffeeSupply implements Supply {
    private $quantity;

    public function __construct($quantity)
    {
        $this->quantity = $this->checkQuantity($quantity);
    }

    private function checkQuantity($quantity)
    {
        if ($quantity === null) {
            throw new InvalidArgumentException("Coffee supplies number must be defined.");
        }
        if ($quantity % 5 != 0) {
            throw new InvalidArgumentException("Coffee supplies must come in multiples of 5.");
        }
        return $quantity;
    }

    public function getBeverageName()
    {
        return 'Coffee';
    }

    public function getQuantity()
    {
        return $this->quantity;
    }
}

<?php
class ChocolateSupply implements Supply {
    public function getBeverageName()
    {
        return 'Chocolate';
    }

    public function getQuantity()
    {
        return 10;
    }
}

Notice how simple the CoffeeMachine class becomes:

<?php
class CoffeeMachine
{
    /**
     * @array
     */
    private $supplies;

    public function __construct(array $beverages)
    {
        $this->supplies = array();
        foreach ($beverages as $name) {
            $this->supplies[$name] = 0;
        }
    }

    public function loadSupplies(Supply $supply)
    {
        $beverageName = $supply->getBeverageName();
        $quantity =  $supply->getQuantity();
        $this->supplies[$beverageName] += $quantity;
    }

    public function getSupplies($beverageName)
    {
        return $this->supplies[$beverageName];
    }
}

The tiny refactorings have paved the way for getting to see the underlying domain concept: the Supply object, which we have transformed in a first-class citizen only by extracting already existing code from the CoffeeMachine class. Supply implementations manage their validation by themselves, and reduce the complexity of CoffeeMachine, which was already getting longer and longer without having made any coffee whatsoever.

Thus, when you are wondering if it's useful to extract a private method, just factor it out: it may end up in another class in the future. If you're wondering if reordering the parameters of several methods for consistency serves a purpose, think that you may end up finding a Parameter object. Don't be afraid of small changes that lead you to a breakthrough.