Code Smells: Static References to Time
Published
So you need to do some time-specific calculations in your application. It’s related to the current time, so you do the usual thing, and new
up a DateTime
object:
$now = new DateTime();
Seems innocuous, right? But you quickly run into some pretty nasty problems:
- Your code is now very difficult to test reliably. To make any assertions on the behaviour that aren’t time-flakey (i.e. they work today but not in a few months time), you’ll now need to do something with the internal system clock while the test is running. Perhaps you can get away with running your tests in a container whose clock is fixed to the desired time, but this is a considerable amount of work to get around the brittleness of the code itself (we will see below how to achieve this same outcome without the need for such machinery).
- If something is “off” with the internal system clock (e.g. the timezone isn’t the one you’d expected) and you need to make a change to your application code, you need to make this change in all areas of the code that directly instantiate the
DateTime
object. That’s potentially a lot of room for error: a bug just waiting to happen.
Your application has time-dependent behaviour, and so it is worth structuring it to make this explicit and easy to manage. The solution? A clock!
interface Clock
{
public function now(): DateTimeInterface;
}
Instead of directly instantiating a DateTime
object, you use a factory which returns DateTime
instances. In this case, our “factory” has a common-sense name: it returns the current time, so it has the behaviour of a Clock
. Pass this as an argument to the method or as a constructor for the service object concerned, and voila:
$this->clock->now();
This cleanly solves both of the issues we saw earlier.
On the testing front, we have two implementations of the clock. The production implementation is straight-forward: just new up a DateTime
object:
final class LocalClock implements Clock
{
public function now(): DateTimeInterface
{
return new DateTimeImmutable();
}
}
For tests, we use an implementation that is most useful for the test criteria. We can implement a clock that always returns the same time:
final class StaticClock implements Clock
{
private DateTimeInterface $time;
public function __construct(DateTimeInterface $time)
{
$this->time = $time;
}
public function now(): DateTimeInterface
{
return $this->time;
}
}
This is great when we want to test behaviour where the clock is only expected to be called once. For example, suppose I have a greeter which greets someone a time-sensitive message. Prior to 12pm it greets them “Good morning”; prior to 6pm it greets them “Good afternoon”, and after then it greets them “Good evening”.
final class Greeter
{
private Clock $clock;
public function __construct(Clock $clock)
{
$this->clock = $clock;
}
public function greet(string $name): string
{
// get the current hour
$hour = (int)$this->clock->now()->format('H');
if ($hour < 12) {
return 'Good morning, ' . $name;
} elseif ($hour < 18) {
return 'Good afternoon, ' . $name;
} else {
return 'Good evening, ' . $name;
}
}
}
Testing this is trivial. For each scenario, we create a clock whose time is fixed to the desired one, and assert that the response is the intended one:
$morningGreeter = new Greeter(new StaticClock(new DateTimeImmutable('2020-01-01 06:00:00')));
$afternoonGreeter = new Greeter(new StaticClock(new DateTimeImmutable('2020-01-01 12:00:00')));
$eveningGreeter = new Greeter(new StaticClock(new DateTimeImmutable('2020-01-01 18:00:00')));
assert('Good morning, stranger' === $morningGreeter->greet('stranger'));
assert('Good afternoon, stranger' === $afternoonGreeter->greet('stranger'));
assert('Good evening, stranger' === $eveningGreeter->greet('stranger'));
In a situation where we call the clock multiple times, we may want an implementation that returns a different DateTime
on each now()
invocation. For example, suppose we were timing a function call:
function secondsTimer(Clock $clock, function $callback): int
{
$started = $clock->now();
$callback();
// in practice you would use greater precision than seconds
// this is just to illustrate the concept
return $started->diff($clock->now())->s;
}
This can be tested with a clock that holds a stack of different times, pulling the first element off the stack with each call:
final class StackClock implements Clock
{
private array $times;
public function __construct(DateTimeInterface ...$times)
{
$this->times = $times;
}
public function now(): DateTimeInterface
{
return array_shift($this->times);
}
}
Just pass in the desired times:
$clock = new StackClock(
new DateTimeImmutable('2020-01-01 00:00:00'),
new DateTimeImmutable('2020-01-01 00:00:03'), // <- 3 second difference
);
$callback = function () {
// do nothing
};
assert(3 === secondsTimer($clock, $callback));
…and you’re done!
Without using a Clock
your only real option for testability would be to use a function that sleep
s for the desired amount of time and hope that nothing untoward happens when running the test:
$callback = function() {
sleep(1);
};
assert(1 === secondsTimer($callback));
With respect to changing our clock on the off chance of a timezone error, we can implement a clock that always returns times in a fixed timezone:
final class FixedTimeZoneClock implements Clock
{
private DateTimeZone $zone;
public function __construct(DateTimeZone $zone)
{
$this->zone = $zone;
}
public function now(): DateTimeInterface
{
return new DateTimeImmutable('now', $this->zone);
}
}
The change is localised (no pun intended) to the clock, and the implementation details of how the time is found are hidden from everywhere else in the code. We have a strategy that maximizes the testability of the code under time-dependent behaviour without sacrificing any functionality, and it wasn’t difficult to implement, either!
As Kevlin Henney succinctly put it:
Time is a resource, just like a file.
The very same strategy for abstracting away the details of file IO from the rest of our code are applicable to abstracting “time IO”.