Bashar’s Blog

I recently watched an interesting presentation from Miško Hevery intituled “The Clean Code Talks - Don’t Look For Things!”. This presentation discusses some best practices to follow in order to keep a code clean. It talks about unit testing and the law of Demeter in OOD.

For the people who never heard about it, the law of Demeter can be succinctly summarized as “Only talk to your immediate friends.” The fundamental notion is that a given object should assume as little as possible about the structure or properties of anything else (including its subcomponents).

The construction of an object should be easy

In his presentation, Miško emphasizes on the importance to make it easy to construct an object. The easiest the construction of an object is, the easier it is to test it.

Let’s look at the following code.
 

<?php

class Document
{
    public $html;

    public function __construct($url)
    {
        $client = new htmlClient();
        $this->html = $client->get($url);
    }
}

The instantiation of a Document object relies on the call to the “get” method of the htmlClient. Testing that class would rely on the htmlClient class and the previous code don’t allow to use any form of mockup object.

What about the following code ? Wouldn’t it be easier to test ?

<?php

class Document
{
    public $html;

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

This code would be much easier to test, this is mainly due to the fact that the instantiation is lighter and easier to perform.

A practical example

Imagine you are in a store and the item you are purchasing is 25$.

• Do you give the clerk your wallet and let him/her retrieve the 25$ ?
• Or, do you give the clerk 25$ ?

The answer seems pretty obvious. Let’s see now how this could be interpreted in PHP.

<?php

class Goods
{
    public $accountReceivable;

    public function __construct($config)
    {
        // Instantiating the accountReceivable
        // using the config
        $ar = new accountReceivable($config);
        $this->accountReceivable = $ar;
    }

    public function purchase(Customer $customer)
    {
        $wallet = $customer->getWallet();
        $money = $wallet->getMoney();
        $this->accountReceivable->recordSale($money);
    }
}

Lets look at the constructor method. It receives a config parameter that will be used to instantiate the accountReceivable object and then assign it to the property of the goods class.

The purchase method receives a customer object. It will retrieve the wallet and the money. Then, it will call the recordSale method and pass the money as a parameter.

So, Why is the previous code sample a bad implementation ?

First, it violates the “law of Demeter” principle by calling getMoney on the wallet ($customer->getWallet()->getMoney()).  This makes objects more dependent on the internal structure of other objects.

Second, this code is hard to unit-test and will require instantiating objects such as the config and customer when being tested, as you can see in the following code.

<?php

class GoodsTest
{
    public function testPurchase()
    {
        // Make it harder to create a mockup object
        $config = new config($fileName);
        $goods = new Goods($config);

        $customer = new customer(new Money(25, "USD"));
        $goods->purchase($customer);

        assertEqual($this->accountReceivable, 25);
    }
}

So, how can we improve this code  ? Let’s look at the following code.

<?php

class Goods
{
    public $accountReceivable;

    public function __construct(accountReceivable $ar)
    {
        $this->accountReceivable = $ar;
    }

    public function purchase(Money $money)
    {
        $this->accountsReceivable->recordSale($money);
    }
}

Why is this code better for unit testing ? First, the constructor don’t rely anymore on the config object. The accountReceivable object is passed directly into the constructor. We will be able to use a mockup object for accountReceivable.

Secondly, the purchase method now receives a money object, instead of the customer. It receives exactly what it needs in order to perform a purchase. We don’t rely anymore on the getWallet() and getMoney() methods.

Our testing class could look like this.

<?php

class GoodsTest
{
    public function testPurchase()
    {
        // Using a mockup object
        $ar = new accountReceivableMockUp();

        $goods = new Goods($ar);
        $goods->purchase(new Money(25, "USD"));

        assertEqual($this->accountReceivable, 25);
    }
}

Use of pre-conditions inside a constructor

Another concerns raised Miško with unit-testing is the use of pre-condition inside constructors. Let’s look at the following code.

<?php

class House
{
    public function __construct($window)
    {
        if(null === $window)
        {
            throw new Exception();
        }
    }
}

 
How the previous could make unit testing harder ? Let’s assume that there is a paint method that receives a color as a parameter. The test of that single method would actually require to pass a not null parameter into the house constructor. So, it wouldnt be possible to do the following.

<?php

$house = new house(null);
$house->paint("red");

Singleton and unit-testing

The use of singleton can potentially make unit testing harder. Let’s assume we have a printer class that implements a method named print. If the passed parameter is null, we would like to log a message.

<?php

class printer{

    public function print($document = null)
    {
        if(null === $document)
        {
            // Retrieve Singleton logger
            $logger = logger::getInstance();
            $logger->logMessage("Print Error!");
        }

        // ...
    }
}

The print method will be hard to unit-test since it uses the logger singleton object. The unit test is no longer contained within known boundary conditions and may generate false results. Also, in the previous code, there is no easy way to substitute a mock object for the logger in a unit test to establish a known boundary condition.To overcome that problem, dependency injection can be used. The following code is an example.

<?php

class printer{

    public function print($document = null, $logger)
    {
        if(null === $document)
        {
            $logger->logMessage("Print Error!");
        }

        // ...
    }
}