Wednesday, November 04, 2009

Testing and constructors

During unit tests preparation you often have to modify your design to simplify the test code. Design for testability is one of the good things about Test-Driven Development and more in generally of unit testing (and with TDD you will not modify your code to allow easier testing but you will write it to do so). A testable design is decoupled and maintainable and in the long run you will get only advantages. With in the long run I intend next week.
One of the pillars of design for testability, which is listed also in the Google guide for code reviewers, is to have constructors that do not contain logic or method calls. I will explain the reasons behind this choice with an example, but let me say that to reduce dependencies it is natural for a class to avoid making assumptions on the external environment, performing things like creating new objects. Separating the business responsibility of a class from the one of wiring itself to other objects is necessary.

As always, the example is in php as there is a great need for good object-oriented programming in this particular field.
class NakedEntity
{
    protected $_entity;
    protected $_class;

    public function __construct($entity = null, NakedEntityClass $class = null)
    {
        $this->_entity = $entity;
        $this->_class = $class;
    }

    public function getState()
    {
        $state = array();
        foreach ($this->_class->getFields() as $name => $field) {
            $getter = 'get' . ucfirst($name);
            $state[$name] = $this->_wrapped->$getter();
        }
        return $state;
    }

    public function getMethods()
    {
        return $this->_class->getMethods();
    }

    // other methods...
}
I omitted docblocks as they are not relevant in this context.
This class purpose is to act as a container for a domain object and its class metadata. These metadata are kept in its $_class field which is a NakedClass object. The extraction process uses reflection and it is encapsulated in another component that returns instances of NakedClass and NakedEntity. Thus, creating a test-friendly NakedEntity object is simple:
$entity = new NakedEntity(new stdClass, new NakedClass('My_User', $methods));
This code snippet tricks my library classes in believing that a stdClass is a My_User istance, and makes unit tests very simple. For instance the classes that work on the methods metadata can try many different combinations of methods without requiring me to write many My_User and My_Group classes. I should thank my constructor: it does not perform work, but only accepts the collaborators and stores them in private fields. If the constructor of NakedEntity parsed the class code like this:
$user = new My_User();
$entity = new NakedEntity($user); // calling get_class() and then doing a lot of work
I would be stuck with My_User methods and I would have to write a new real class for every different test method and my tests would be slower.

Fortunately there is mocking, and I can substitute NakedEntity instances with mocks. But mocking won't always save me from the work in the constructor, as when testing the very NakedEntity class I will have to wait for all the parsing to finish in every test, while I just want to verify that, given a set of method metadata getMethod() and hasMethod() works well. Why should my classes parse docblocks and annotations to test ten lines of array-related code?
Unit testing also means that the code under test is contained in the NakedEntity class, and I can inject mocks or stubs via the constructor instead of a real entity or a real NakedEntityClass object. Keep in mind also that a stub can be the the real production class if a test-friendly instance can be built and it has not much logic: in other tests I often use the NakedEntity class, and I will continue at least until it grows to contain logic I want to take out of my other unit tests.
Focusing on the NakedEntityTest class instead, if the constructor created objects or did a lot of logic it would be difficult to inject the right stubs as they can get in the way during the costrunctor execution. Also a problem in the constructor would make all the tests fail, making more difficult to locate the problem.
One acceptable practice is to instance only newable objects in the constructor, with little behavior. In php an array is the obvious example of newable, since it is not even an object. Also I considered Spl classes newable as they do not slow down tests and cannot commonly break (at least you cannot break them while maintaining your code).

Note the null defaults that the arguments assume, that allow tests to pass in nulls instead of real objects or mocks. I think this is useful when sometimes a collaborator is not used in mediator objects:
$entity = new NakedEntity(null, $class); // emphasis that the test would
// use the NakedClass collaborator.
$methods = $entity->getMethods();
The contract in the code above says "getMethods() should not assume anything about the $this->_entity property". If getMethods() tries to call a method on a null, the code will explode and the test will fail.
It is responsibility of the factory class to create a valid object by passing in meaningful collaborators instead of nulls, and this responsibility would be exercised in an integration or functional test.

In my opinion this is the perfect constructor for my class:
  • it lets the tests inject collaborators, which could be real instances, mocks or stubs;
  • it does not perform work that should be repeated in every test; it only assigns variables to private or protected fields;
  • it lets the collaborators be null values if they should not be touched during a particular scenario.
I know someone will complain: but now how I can write new NakedEntity(...) to work with an instance? I will have to create all its collaborators. You should not do it. A factory should do it for you, and if you require to do it in the middle of a method, you should ask for a factory in the related constructor.
Do you have some example of constructor difficult to refactor in this way? Feel free to post it on pastebin.com and insert the link in the comments for discussing it.

No comments:

ShareThis