Discharging Static #1

It’s been seven years since Kore Nordmann first published “static considered harmful” on his blog, explaining the problems with using static method calls in classes, and the difficulties that they cause when trying to test that class. Seven years on, and those difficulties are still the same, and there is still new code being written using static calls despite that knowledge; but it’s often a more severe problem in legacy code with little or no unit tests.

So why exactly are static calls so bad? If you’ve read Kore’s article, then you probably have a good idea already; but what that article doesn’t cover is approaches that we can use to make the code testable.

The main issue with static methods is that they introduce coupling by hard-coding the dependency into your code, making it difficult to replace with stubs or mocks in your Unit Tests. This violates the Open/Closed Principle and the Dependency Inversion Principle, two of the principles of SOLID.

Testing static methods themselves is normally fairly straightforward, as long as they are “leaf” methods that make no static calls of their own that would need mocking: the real difficulties arise when we need to mock static methods (either when called within instances, or when they are non-leaf methods in a chain of static calls); or where static properties are used to maintain state.


There was a short time when PHPUnit provided a staticExpects() method for creating mocks for static calls; but it came with restrictions to its use that developers rarely understood or followed, and didn’t work as they expected, so since PHPUnit 4.0, it has no longer been available. Other libraries like Mockery, Phake or AspectMock do provide solutions, they still have their own difficulties.

Phake can only stub calls to code that uses late static binding, because that is determined at run time, whereas calls using self or the class name are determined at compile time. This is basically the same as those earlier versions of PHPUnit that allowed mocking of static methods, and part of the reason why Sebastian Bergmann removed those methods from PHPUnit was because developers didn’t understand that limitation.

Mockery (alternatively) supports static mocking using class-aliased mocks, and works by intercepting the autoloader calls and blocking autoloading of those aliased classes, so that it can create its own stub and direct the call towards this instead. But Mockery is designed to help when running standalone tests rather than a full unit test suite. It does provide the Mockery::close() method that we can put in the tearDown() for our PHPUnit tests to clean up the Mockery container, or we can extend our tests from \Mockery\Adapter\Phpunit\MockeryTestCase, or use the \Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration trait; but that approach of intercepting the autoloader can still cause problems with other tests in the suite that might use PHPUnit mocks of the same class, causing those tests to fail. If you’re working as part of a team rather than as a sole developer, then that is something you may not be able to control; and while I find Mockery incredibly powerful and useful, it still isn’t a perfect solution for testing code that uses static methods.

I’ve only just started looking at using AspectMock, but it really does look very useful and simple, and does provide support for running it with PHPUnit; but I haven’t really used it enough yet to comment on its benefits and drawbacks (perhaps a few weeks experimenting with it will lead to a future blog post redressing that).

In the meanwhile though, there are a couple of alternative approaches that we can take when testing code that makes static calls.


Let’s start with a simple class we want to test that will return a date range, in this case we want to test the weekToDate() method:


class dateCalculator {
    protected $currentDate;
    protected $logger;

    public function __construct() {
        $this->currentDate = new \DateTime();
        $this->logger = \Logger::getInstance();
    }

    public function weekToDate() {
        $startDate = clone $this->currentDate;
        $startDate->modify('last monday');
        $this->logger->info('Week to date: ' . $startDate->format('Y-m-d'));
        return new dateRange(
            $startDate->modify('midnight'),
            (clone $this->currentDate)->modify('midnight')
        );
    }
}

There are actually two problems with this class – both in the constructor – that make it difficult to test. The use of new to instantiate a dependency, and the use of a static call to get a logger instance. If we could rewrite this class to dependency inject the currentDate value and the logger, then there would be no problems; but that rewrite would entail changing everywhere in the system that the dateCalculator was instantiated, or it would require introducing some other dependency injection mechanism into the system, which would entail a major change to the existing architecture of the application. Neither change is good without a decent suite of unit tests to validate those changes in the first place. So we have a chicken/egg situation: we can’t easily test without changing the code, but it’s risky to change the code without having tests.

As an initial approach, we can use one of my favourite techniques, create an anonymous class (which does require PHP >= 7.0) that extends our dateCalculator class, and that overrides the constructor, allowing us to create a new constructor that can be Dependency Injected.


/**
 * @dataProvider testDateCalculatorWeekProvider
 **/
public function testDateCalculatorWeek($baseline, $expectedResult) {
    $logger = new class($this) {
        protected $logToTestClass;
        public function __construct($logToTestClass) {
            $this->logToTestClass = $logToTestClass;
        }
        public function info($message) {
            $this->logToTestClass->logArray[] = $message;
        }
    };

    $dateCalculator = new class($baseline, $logger) extends dateCalculator {
        public function __construct($baseline, $logger) {
            $this->currentDate = new \DateTime($baseline);
            $this->logger = $logger;
        }
    };

    $week = $dateCalculator->weekToDate();
    .... Assertions
}

Creating an anonymous class to override the constructor of the class being tested means that we can’t test the constructor itself, but all the original constructor is doing is setting the currentDate and logger values for use in the methods that we do want to test, and it doesn’t contain any actual logic – sane constructors shouldn’t contain logic anyway, otherwise that creates its own testing issues. This then allows us to inject a whole set of different dateTime values so that we can run comprehensive tests against the weekToDate() method; and it allows us to record the messages sent to the logger so that we can also test that those are correct. (Testing of the Logger itself should be a separate test against the Logger class.) In doing this, we have decoupled the dependency of the dateCalculator class from DateTime and Logger as regards testing; though our code coverage won’t include the constructor because we’re overriding with the constructor code in our anonymous class. But once we have built a suite of tests for the application as a whole, we will be in a position to start refactoring it to use dependency injection and other SOLID principles, and then rewriting our tests accordingly.


A second approach – which involves some code change, though only a minimum of refactoring to the class that we’re testing, and no modification to code outside of that class – is to modify that static call to an instance method call, and call a date provider Accessor method, both of which can be mocked:


trait CallStatic {
    protected function callStatic($className, $methodName, ...$parameters) {
        return ($className . '::' . $methodName)(...$parameters);
    }
}

class dateCalculator {
    use CallStatic;

    protected $currentDate;

    protected function getCurrentDate() {
        return new \DateTime();
    }

    public function __construct() {
        $this->currentDate = $this->getCurrentDate();
        $this->logger = $this->callStatic(Logger::class, 'getInstance');
    }

    ....
} 

We can then mock the getCurrentDate() and callStatic() methods directly, and our code coverage will include the constructor, and will only miss the mocked getCurrentDate() method and the code within the Trait (which we are also mocking).

This latter approach is particularly useful when we have static calls in the class methods that we want to test, so code like:


public function weekToDate() { 
    $startDate = clone $this->currentDate;
    $startDate->modify('last monday');

    Logger::info('Week to date call: ' . $startDate->format('Y-m-d'));

    return new dateRange(
        $startDate->modify('midnight'),
        (clone $this->currentDate)->modify('midnight')
    );
}

where we aren’t using a logger instance, but calling a static logger info() method, can be modified to use our callStatic() method from the Trait instead.


public function weekToDate() { 
    $startDate = clone $this->currentDate;
    $startDate->modify('last monday');

    $this->callStatic(Logger::class, 'info', 'Week to date call: ' . $startDate->format('Y-m-d'));

    return new dateRange(
        $startDate->modify('midnight'),
        (clone $this->currentDate)->modify('midnight')
    );
}

Creating Assessor methods just to simplify testing isn’t the cleanest solution; but it does provide an interim solution, allowing us to make the class testable until such time as we can refactor and improve the code in a way that allows us to test it without such hacks.


So there we have a couple of approaches that allow us to make methods containing static calls more easily testable.

Static properties create rather different problems for testability, but I’ll address those issues in a future post.

Advertisements
This entry was posted in PHP and tagged , , . Bookmark the permalink.

5 Responses to Discharging Static #1

  1. Pingback: Marc Baker: Discharging Static #1 - webdev.am

  2. Pingback: Discharging Static #2 | Mark Baker's Blog

  3. Pingback: Aspects of Love — How deep does the rabbit hole go? | Mark Baker's Blog

  4. Andre2999 says:

    Excellent article, I have learned quite a lot, thank you.

    Like

  5. Pingback: Marc Baker: Discharging Static #1 | Premium apps reviews Blog and Programing

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

w

Connecting to %s