about me about me
photography photography
software projects software projects
contact me contact me
10
Mar

This might sound heavy handed and radical but it’s widely accepted, particularly among the testing community that statics are evil. Specifically I’m referring to static properties and methods, with exception to self for late static binding and parent used in inheritance.

At first glance, statics are a convenience but as your code base grows, and especially if you try to write unit tests around your code, the convenience turns into pain. Why?

  • Static methods are anti-OOP and increase coupling
  • Static methods eliminate seams in your software
  • Statics are not required in PHP, we have global application state
  • Statics are the foundations for patterns prone to side effects

Convenience

Some of these problems are not immediately apparent, a mind share of programmers will see static methods as call time convenience. Those that recognised these issues, steer clear of them. So essentially, they’re a “gotcha”.

PHP 5.4 has arrived with support for instance method calls, this means static call time convenience can be matched by instance methods.

// static one liner
$user = User::load($userId);

// vs instance
$user = new User;
$user->load($userId);

// PHP 5.4
$user = new User()->load($userId);

However, inline instantiation isn’t buying you much here. I don’t advocate sprinkling new throughout your code. My example only serves to demonstrate how static calls can be replaced by equivalent one line instance method calls.

Lets describe each of the disadvantages above in more detail.

Static methods are anti-OOP

Hopefully this is pretty obvious. You are throwing away the power of object oriented programming and are left with namespaced functions.

// static method
class User {
    public static function load($userId) { ... }
}

// namespaced function
namespace User;
function load($userId) { ... }

// usage comparison
User::load($userId);
\User\load($userId);

We’ve just hard coded a coupling to a specific implementation.

Can we make use of polymorphism here? No
Can we subclass User and use inheritance here without changing the calling code? No
Can we encapsulate some static but still globally accessible state? Yes

Hooray, we’ve just reverted to procedural programming!

Static methods eliminate seams in your software

What’s a seam you might ask? A seam is where you’ve decoupled your code, and can substitute one implementation for another. Introducing seams in software is good design, they facilitate mocking / stubbing (crucial for unit tests).

Code with seams is less likely to need refactoring to accommodate change, as the collaborators are interchangeable. If it does require refactoring, less code will need to be modified in a decoupled system. Lets look at an example to help clarify this reasoning.

class User {
    protected $id;
    protected $model;

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

    public function getId() {
        return $this->id;
    }

    public function load($userId) {
        $data = $this->model->loadById($userId);

        if (empty($data)) {
            throw new UnexpectedValueException(
                'No user found with ID ' . $userId
            );
        }

        foreach ($data as $attr => $value) {
            $this->$attr = $value;
        }
    }
}

And the corresponding tests.

class UserTest extends PHPUnit_Framework_TestCase {
    protected $userId = 1;
    protected $model;

    public function setup() {
        $this->model = $this->getMock(
            'Model_User', array('loadById'), new Zend_Test_DbAdapter
        );
    }

    public function testLoadUserIdOne() {
        $dbRow = array('id' => $this->userId);
        $this->model->expects('loadById')
                    ->with($this->userId)
                    ->will($this->returnValue($dbRow));

        $user = new User($this->model);
        $user->load($this->userId);
        $this->assertEquals($this->userId, $user->getId());
    }

    /**
     * @depends testLoadUserIdOne
     * @expectedException UnexpectedValueException
     */
    public function testLoadInvalidUserIdThrowsException() {
        $this->model->expects('loadById')
                    ->with($this->userId)
                    ->will($this->returnValue(array()));

        $user = new User($this->model);
        $user->load($this->userId);
    }
}

So this code is a little contrived (should User really know about it’s model?) but we can pull all the levers outside it to exercise all conditions, test for failure and success. It doesn’t require a database to run. Lets rework this to call static methods on Model_User.

class User {
    protected $id;

    public function getId() {
        return $this->id;
    }

    public function load($userId) {
        $data = Model_User::loadById($userId);

        if (empty($data)) {
            throw new UnexpectedValueException(
                'No user found with ID ' . $userId
            );
        }

        foreach ($data as $attr => $value) {
            $this->$attr = $value;
        }
    }
}

This change looks harmless? Lets look at our tests …

class UserTest extends PHPUnit_Framework_TestCase {
    protected $userId = 1;

    public function testLoadUserIdOne() {
        $user = new User;
        $user->load($this->userId); // ah, we need a DB server here :(
        $this->assertEquals($this->userId, $user->getId());
    }

    /**
     * @depends testLoadUserIdOne
     * @expectedException UnexpectedValueException
     */
    public function testLoadInvalidUserIdThrowsException() {
        $user = new User;
        $user->load($this->userId); // ah, we can't force a failure
    }
}

Aside from requiring a database server, we don’t have any handle on Model_User anymore. In a real application, I guarantee you’ll run into the situation where you won’t be able to induce failure and test error handling or a branch of an if ... else statement.

It’s unfair to lay blame solely on static methods. You can find yourself in this situation with instance methods, if you instantiate your collaborators within your class. But … using static methods takes away the opportunity to fix it by passing in collaborators!

PHP has global application state

The global application state / namespace in PHP is what makes the language initially accessible.

<?php

$msg = "I come from global state";
echo $msg;

function foo() {
    global $msg; // we all know this is bad
    ...
}

class Bar {
    public function baz() {
        global $msg; // this too
    }
}

We know the global namespace is not where you want to put your application code. Java has no global namespace by design. The entry point of a Java application:

class MyApp {
    public static void main(String args[]) {
        String msg = "I come from main";
        System.out.println(msg);
    }
}

In Java, without going to disk or over the network, accessing global state is made possible via static properties and methods (but for the same reasons cited above they’re frowned upon). You can see from PHP 5 syntax, that it borrows heavily from Java but PHP doesn’t need statics for this purpose.

Statics are the foundations for patterns prone to side effects

The Singleton and Registry patterns are only possible due to statics. They’re also considered anti-patterns.

… if I instantiate two objects A and B, and I never pass a reference from A to B, then neither A nor B can get hold of the other or modify the other’s state. This is a very desirable property of code. If I don’t tell you about something, then it is not yours to interact with! Notice how this not the case with global variables or with Singletons. Object A could — unknown to developers — get hold of singleton C and modify it. If, when object B gets instantiated, it too grabs singleton C, then A and B can affect each other through C.

Misko Hevery

In summary, fewer poor design choices would be available to developers if these features were stripped from PHP. What is the likelihood of this happening? Close to zero, certainly not before PHP 6 as this would break backwards compatibility. I hope from now on you, like me, avoid static properties and methods.


You can skip to the end and leave a response. Pinging is currently not allowed.

comments

Ian Rogers // 16.Mar.2012 // 15:00

Static methods are just namespaced procedures, as you rightly say. But you shouldn’t criticise them for that because, in fact, that’s *all* they’re good for :-)




this post's tags