Unit Testing magical code

2012-11-23

Simply put, unit testing code with lots of magic is hard, much harder than it needs to be. However, if you seek out the magic in your code and replace it with clear, explicit calls to well-defined functionality, testing becomes much, much easier.

Take this common use of magic functionality: __get. This is often used to create fake public properties on an object. Probably the most egregious use of this magic method (that I've seen multiple times in various codebases) is code like this:

public function __get($name)
{
    $propName = "_$name";
    if (isset($this->$propName) {
        return $this->$propName;
    }
}

This basically says that if there's a property named with the (somewhat dated) convention of prefixing protected/private properties with underscores, then just return the named property. This removes any encapsulation/information hiding benefit of using protected/private in the first place and lets any outside user of the class get at the class's tasty, gooey insides without going through the normal checks and balances that prevent that access.

So in addition to undoing any real reason to make a property protected/private (especially when combined with a similar __set method), you've made the code less explicit to users and also made it slower... a magic method call in PHP is (in general) around 3x slower than a standard method call. Admittedly, we're dealing with fractions of a second, but considering the following:

  • Every magic property access adds up. How many times to you access properties in a typical view? Do you access these properties inside loops?
  • Code becomes harder to introspect, which interferes with code completion
  • The code becomes harder to understand for new users (or yourself in 6 months)

Testing

How would you test the following code, knowing that a __get was involved? if ($this->foo->bar->baz === 7) { ...do stuff... } In my view this code has two main problems:

  • It is clearly violating the Law of Demeter by digging deeply into neighboring objects
  • Property access (especially with __get) is more complex/unintuitive to mock than function access

In order to test fake property code with __get like that, we need a mock like this:

$mockBar = $this->getMock('Bar');
$mockBar->expects($this->atLeastOnce())
    ->method('__ get')
    ->with('baz')
    ->will($this->returnValue('some mock of Bar'));
$mockFoo = $this->getMock('Foo');
$mockFoo->expects($this->atLeastOnce())
    ->method('__get')
    ->with('bar')
    ->will($this->returnValue($mockBar));

It gets even more complicated if you have multiple fake properties accessed during the same test, then you have to use something like returnValueMap or returnCallback, which can get ugly fast.

Conclusion

In my view, you're better off using explicit getters and setters for object properties, unless it is a simple value object, then public properties can still make sense. It may seem like more code to write, but that type of code is easy to generate automatically via an editor or a simple script, and you only have to generate it once. In the end, you are left with code that's faster and more explicit, and you can make specific decisions about whether you want, for example, a property to be editable or not by not providing a setter.

Resources

Tags: php, phpunit

Comments