Do We Use Magic Methods or Not?

PHP-logo

Are magic methods in PHPand by consequence Zend Framework, worth using? When you use them, can you truly write testable code? Let’s explore one side of the argument today.

As a freelance Zend Framework developer, I’m always looking to improve the quality of the applications I produce. So over the last 6 – 12 months, I’ve been learning as much as possible about testing. During this time, I’ve found the way I code’s dramatically changing (and improving).

Some constructs, both large and small, which I’ve used habitually have been falling by the wayside, as I find out they’re either extremely difficult or impossible to test.

In a recent development session, I attempted to test some of my \Zend\Db based classes, specifically the code which used the magic methods for dynamically building where clauses. Here’s an example:

$select->where->lessThanOrEqualTo("dateOfBirth",
    new \Zend\Db\Sql\Expression(
        sprintf("STR_TO_DATE('%s','%%d-%%m-%%Y')", $searchCriteria->getValue("endDate"))
    )
);

Here, I’ve called lessThanOrEqualTo on the dynamic property where to find records which have a dateOfBirth no later than the formatted date supplied. The lessThanOrEqualTo method is accessed through a dynamically exposed where property made possible by an implementation of the magic method __get in \Zend\Db\Select. You can see the function implementation below:

/**
 * Variable overloading
 *
 * @param  string $name
 * @throws Exception\InvalidArgumentException
 * @return mixed
 */
public function __get($name)
{
    switch (strtolower($name)) {
        case 'where':
            return $this->where;
        case 'having':
            return $this->having;
        default:
            throw new Exception\InvalidArgumentException(
                'Not a valid magic property for this object'
            );
    }
}

You can see it will return $this->where, which is a Zend\Db\Sql\Where object, which in turn extends Zend\Db\Sql\Predicate\Predicate. Ok, the levels of indirection aside, it’s not that complex. Personally, I like the simplicity and flexibility of it.

But Is It Testable?

I can’t speak for what it’s like using PHPUnit’s mock objects, as I always use Mockery instead. But after attempting to do so in Mockery, I hit a stumbling block when trying to test the chained call. Not being able to resolve it, I turned to #zftalk on IRC and after some discussion there, I revisited the manual to find the following:

PHP magic methods which are prefixed with a double underscore, e.g. _set(), pose a particular problem in mocking and unit testing in general. It is strongly recommended that unit tests and mock objects do not directly refer to magic methods. Instead, refer only to the virtual methods and properties these magic methods simulate.

Following this piece of advice will ensure you are testing the real API of classes and also ensures there is no conflict should Mockery override these magic methods, which it will inevitably do in order to support its role in intercepting method calls and properties.

This lead me to start re-evaluating my attitude to PHP’s magic methods. Whereas previously I’ve loved using them for their simplicity and compactness, now I’m not so sure the convenience is worth it.

As the manual suggests, “mock on the properties which the magic method refers to”. Fine, not a deal breaker. But perhaps it’s time to stop using them. Maybe it’s time to write more getters and setters instead, in the name of fully testable code. Maybe, whilst handy, they’re just not worth using.

During research on the topic, I came across this stackoverflow article, which in the comments, people make good arguments for and against. Here’s 3 examples:

Also, magic methods aren’t going to work with interfaces and abstract classes/methods. And what about visibility? It’s a leak to have a private or protected property modifiable by a magic __set method. Add these to your examples and I agree that there are many good arguments against using magic methods for getter/setters and no real arguments in their favor. For quick one-offs, maybe use them for something, but they’re not a best practice for reusable code/libraries/APIs. – joelhardi May 31 ’11 at 8:47

You should use stdClass if you want magic members, if you write a class – define what it contains.

Yes, this is exactly that :p. But another thing I forgot to mention is that: having getters for properties is more coherent with “real” methods where getXXX is not only returning a private property but doing real logic. You have the same naming. For example you have $user->getName() (returns property) and $user->getToken()

Here’s an excerpt from the article which triggered the post:

First of all, back then it was very popular to use PHP’s magic functions (__get, __call etc.). There is nothing wrong with them from a first look, but they are actually really dangerous. They make APIs unclear, auto-completion impossible and most importantly they are slow. The use case for them was to hack PHP to do things which it didn’t want to. And it worked. But made bad things happen.

Interestingly, in the interests of speed, Google suggests to:

avoid writing naive setters and getters

Instead, they suggest declaring class variables with public visibility. Though, from my interpretation of the post, that’s only for the case of simple get and set, not where extra logic is required.

What say you?

I’ve decided to reevaluate using them for the time being and be more judicious in the use of them. What about you? Where do you stand? Do you avoid them, or can you not live without them?

I’ll be fair, I’ve not answered one of the key questions I posed earlier: But is it testable?. Don’t worry, I’m not avoiding doing so – I’ll do that in an upcoming post. For now though, share your thoughts in the comments.

PHP