Legacy code concepts I: Sensing and Separation

Hi again,

I’ve been lately reading my last technical book acquisition: Working effectively with legacy code (Michael C. Feathers) and, after several chapters, I’ve decided to write down some key concepts in order to make them clearer on my mind.

Context

The book is mainly focused on how to get a class into a “test harness”, or, in other words, how to  successfully create unit tests for legacy code so the risk of side effect changes is minimized. Usually, in order to get a test harness, dependencies between classes must be broken, hence, some code refactors are put in place, this situation leads to a potential vicious circle scenario:

A change -> test harness -> break dependencies (refactoring) -> A change

Breaking dependencies is the cornerstone of the whole strategy for getting a class into a test harness. However, the solution is, as we already know, quite difficult to implement sometimes, so, in order to tackle such challenge, the author proposes a variety of techniques based on his own experiences, although mostly in C++ and Java, all of them 100% applicable to any high level language.

Sensing

Firstly, Michael defines the concept sensing as the ability to know (or sense) how the code, which has to change, can be affected by other class methods and attributes’ value changes. As he points out “we break dependencies to sense when we can’t access values our code operates”.

The main technique explained in order to sense is the use of Fake Collaborators, also Mock Objects. A Mock Object is a Fake Collaborator that also performs assertions.

Faking objects

Say we have the following code:

class Invoice
{
    public function getShippingVat(): float
    {
        if($this->deliveryMethod !== null) {
            return array_sum(array_map(function(ICharge $charge) {
                return round($charge->getCost() * ($charge->getTaxValue() / 100), 2);
            }, $this->deliveryMethod->getCharges()));
        } else {
            return 0;
        }
    }
}

It is our aim to create a test for the method getShippingVat, but, if we execute the method as is, we are also performing an operation on the object deliveryMethod, more precisely, we are calling its method getCharges. We cannot sense getCharges because it computes Charges obtained from a source that depends on how we bootstrap our application or, no need to go that far, the Invoice constructor.

class DeliveryMethod
{
    public function getCharges(): array
    {
        if(empty($this->vCharges)) {
            $this->vCharges = $this->proxy->loadCharges(); // either file or database
        } 
        return $this->vCharges;
    }
}

We can figure out what getCharges does, but we don't want, by any means, to waste any time reading from a file or database, it's not the subject of the test, so, in order to keep the test going, we could just mock the getCharges result.


In a nutshell, the strategy here is to fake the DeliveryMethod and get the corresponding result  from getCharges to continue the test asap.

So, hands on. First, we need to break dependencies by injecting the object via parameter:

public function getShippingVat(DeliveryMethod $deliveryMethod)
    {
        if($deliveryMethod !== null) {
            return array_sum(array_map(function(ICharge $charge) {
                return round($charge->getCost() * ($charge->getTaxValue() / 100), 2);
            }, $deliveryMethod->getCharges()));
        } else {
            return 0;
        }
    }

This change is very agressive and may have a huge impact, as we are changing the method signature. This approach can be problematic and very risky, so, let's take a less invasive approach: we will delegate all the logic to a new method which uses the new signature and we'll invoke our new method inside the old one, see below:

public function getShippingVat()
    {
        return $this->getShippingVatBy($this->deliveryMethod);
    }

    public function getShippingVatBy(DeliveryMethod $deliveryMethod)
    {
        if($deliveryMethod !== null) {
            return array_sum(array_map(function(ICharge $charge) {
                return round($charge->getCost() * ($charge->getTaxValue() / 100), 2);
            }, $deliveryMethod->getCharges()));
        } else {
            return 0;
        }
    }

So now our test will be on the new signature method, which very kindly allows us to inject DeliveryMethod. Moreover, we can think of using an interface so we can inject any object that is DeliveryChargeHolder compliant.

interface DeliveryChargeHolder {
    public function getCharges();
}

class DeliveryMethod implements DeliveryChargeHolder
{
    [...]
}

class FakeDeliveryMethod implements DeliveryChargeHolder
{
    public function getCharges()
    {
        return [
            new Charge(12, 20.00),
            new Charge(6, 20.00),
            new Charge(3, 5.00),
        ];
    }
}

And this is how our new signature looks:

class Invoice
{
    public function getShippingVat()
    {
        return $this->getShippingVatBy($this->deliveryMethod);
    }

    public function getShippingVatBy(DeliveryChargeHolder $deliveryMethod)
    {
        if($deliveryMethod !== null) {
            return array_sum(array_map(function(ICharge $charge) {
                return round($charge->getCost() * ($charge->getTaxValue() / 100), 2);
            }, $deliveryMethod->getCharges()));
        } else {
            return 0;
        }
    }
}

So, eventually, we can create a fake object that has a method which return the expected value:

class InvoiceTest extends PHPUnit\Framework\TestCase
{
    public function testGetShippingVat()
    {
        $invoice = new Invoice();
        $expectedCost = 3.75; 
        $actualCost = $invoice->getShippingVatBy(new FakeDeliveryMethod());
        $this->assertEquals($actualCost, $expectedCost);
    }
}

Separation

Secondly, he talks about separation when it comes to break high coupled systems dependencies, eg, a class that opens a socket or a database connection. Very complicated and time consuming scenarios to replicate just for the sake of getting the class into the test harness. As he states: “we break dependencies to separate when we can’t even get a piece of code into a test harness”.

Conclusion

In short, both concepts represent the need to alter the class code in order to gain testability, because, the main goal is to have the maximum test coverage along with the minimum test execution time.