Untestable code and how-to improve it

Did you find out what was the key culprit of the untestable code segment in my previous post How to recognize bad code?

If you don’t have the time to spend reading the previous article, here’s the problematic code segment again. Let’s just dive straight in.

<?php
  
namespace \CompanyX;
  
class CokeDispenser {
  
    protected $coinCollector;
  
    public function __construct() {
        $this->coinCollector = new CoinCollector();
    }
  
    public function getCoke() {
        if ($this->coinCollector->collectedAmount() >= 2.0) {
            return 'coke';
        }
    }
}

Testability of the code is defined by the openness of the design and loose coupling of the collaborating classes. In this example we can clearly spot two issues.

1. Actual work in the constructor
2. Collaborating class instantiation within the class

Both of these issues make testing of the CokeDispenser class difficult, but combined they make it nearly impossible. Let’s take a quick look at why this is bad.

Actual work in the constructor

While there is nothing wrong with having code in the constructor of the class, that code should only be operating on the properties of the class and not doing any “real” work.

BAD CODE

<?php
namespace \CompanyX;
  
class CokeDispenser {
  
    protected $coinCollector;
  
    public function __construct() {
        $this->coinCollector = new CoinCollector();
    }
}

This code is clearly doing “real” work and what’s even worse it has side effects. Whenever we instantiate a CokeDispenser class, a CoinCollector object is created as a side effect. Definitely not what we desire or expect from instantiating a CokeDispenser object. Not to mention, that it’s violating the Single Responsibility Principle.

Collaborating class instantiation within the class

Objects are rarely isolated and mostly they collaborate with instances of other classes. In such cases there are some good practices to follow in order to make the classes loosely coupled with their collaborators.

BAD CODE

<?php
 
namespace \CompanyX;
  
class CokeDispenser {
  
    protected $coinCollector;
  
    public function __construct() {
        $this->coinCollector = new CoinCollector();
    }
}

The CokeDispenser class is glued to the CoinCollector class, since that class is instantiated in the constructor. It would be difficult to exchange the “Payment Collector” with some other class like CreditCardProcessor or SmartCardProcessor.

In writing good and loosely coupled code, we strive for loosely coupled designs. Who knows which classes will our class have to collaborate with in the future. Two rules to abide by are:

1. Program to abstractions
2. Ask for collaborators, don’t look for them

To improve our problematic piece of code, we’ll abide by both these rules and create a nicely loosely coupled CokeDispenser.

GOOD CODE

First we’ll abstract the CoinCollector class with creating an interface called PaymentCollector and make CoinCollector implement that interface.

So here’s the PaymentCollector interface.

<?php
 
namespace \CompanyX;
 
interface PaymentCollector {
    public function collectedAmount();
}

Let’s make CoinCollector implement the PaymentCollector interface.

<?php
 
namespace \CompanyX;
 
class CoinCollector implements PaymentCollector {
 
    public function collectedAmount() {
        return $this->collectedCoinsAmount;
    }
}

Lastly let’s change the CokeDispenser class to ask for a PaymentCollector class in its constructor. This way our CokeDispenser clearly asks for the collaborating class and doesn’t attempt to create it on its own. Following the Single Responsibility Principle, the CokeDispensers single responsibility is to dispense Cokes, not create CoinCollectors.

<?php
 
namespace \CompanyX;
 
class CokeDispenser {
 
    protected $paymentCollector;
 
    public function __construct(PaymentCollector $collector) {
        $this->paymentCollector = $collector;
    }
 
    public function getCoke() {
        if ($this->paymentCollector->collectedAmount() >= 2.0) {
            return 'coke';
        }
    }
}

This way our CokeDispenser class can easily be tested using a DummyPaymentCollector class that implements the PaymentCollector interface.

I hope this example was easy enough to follow how we turned a badly designed CokeDispenser class that was bound to the CoinCollector collaborator class and turned it into a testable and extendable class.

When you look at it now, it all makes sense. A CokeDispenser needs some sort of a PaymentCollector (coins, bank notes, credit cards, smart cards, …) and our improved class can support them all without any needed modifications for whatever the future brings (i.e.: ImplantedChipPaymentProcessor, BitCoinCollector …).