(or, You Could Have Invented Dependency Injection)
I've been refactoring an existing system from a prototype to a well-designed
and maintainable system. Part of that process is finding a better design. Part
of that process is improving test coverage to find and fix bugs.
Something fascinating happens during that process if you pay attention.
Because the design of the project is fluid at this point, I have free reign
to redesign to make testing easier. Because test coverage is spotty in places,
redesigning to make testing easier is almost a necessity. Fortunately, the
project is small enough and communication with the stakeholders easy enough
that determining what the code should do where it's not clear is
easy.
Plenty of people criticize test-driven development as if it were an
iron-clad rule of law, where every line of code must have at least one
associated test case, and if you do that, you get a badge and some grape
flavored juice to hold you over until the spaceship comes. If that were the
only benefit, that would indeed be a fair criticism—but how do you test
this code?
sub save_to
{
my ($self, $dirpath) = @_;
my $filepath = File::Spec->catfile( $dirpath, $self->cached_file );
my $i = Imager->new( file => $self->full_file );
return $self->invalidate() unless $i;
my $width = $i->getwidth;
my $height = $i->getheight;
# verify size
return $self->invalidate()
if MyCoolApp::Exclude->image_size( $width, $height );
my @resize = $self->should_resize( $width, $height );
try
{
$i = $i->scale( @resize ) if @resize;
$i->write( file => $filepath );
unlink $self->full_file;
$self->state( 'FETCHED' );
$self->full_file( $filepath );
}
catch { warn $_; $self->invalidate };
return 1;
}
This code will not be trivial to test. Perl 5 has a few idioms for testing
the existence of files in the filesystem. With some careful use of File::Temp, it's possible
to verify that a file which did not exist before the call being tested exists
after the call, but the rest of this code has other difficulties.
The hardest part of this code to test comes from the hard-coded call to the
Imager constructor. I
could hijack the loading of Imager with Test::MockObject::Extends
and test all of the paths through this method by changing the mock Imager's
behavior, but that would couple of the details of the test too tightly to the
details of the code being tested.
This method's biggest problem is that it does too much. It creates an Imager
object. It tests to see if the image represented should be excluded from
saving. It resizes the image. Then, finally, it saves the image. All of this
behavior made sense when the only operation performed against this API was
"save this image, if applicable" but these operations are very obviously
distinct operations confused in one spot when trying to test this
API.
This is where the people who dismiss TDD as a only brain-numbing checklist
of rote behavior are wrong. I'd never let myself write a method with this many
separate concerns if I had written it with TDD, but I can admit that I'd write
code with this many separate concerns if I'd written prototype, just get it
working, see of it's possible code. (I did write the exclusion
code.)
Years ago I would have forged ahead to dummy up an entire Imager, just to
get full test coverage. Fortunately, this code uses Moose.
Moose?
The design patterns people have a term called Inversion of Control and a
specific grouping of techniques in a family called Dependency Injection. You
can get lost in the architecture astronaut nature of the discussion, but the
basic principle is sound: avoid hard-coding dependencies in your code.
In other words, the most suspect part of this method is the line:
my $i = Imager->new( file => $self->full_file );
... because of the very tight coupling between this module and Imager.
You may think now "Wait, but you need the behavior of
Imager for the code to work correctly!" and you're correct to do so. That's
absolutely right. This code needs the behavior of the Imager object,
and this code is tied to the API of the Imager object, but this code itself
does not have to be responsible for instantiating the Imager
object.
If you already knew that, give yourself a gold star. If you're now thinking
"Yeah, but why does that matter?" or "What?", read the previous paragraph
again. It's subtle, but it's important.
Moose supports something I'll call synthetic attributes. They're attributes
of your object built from the values of other attributes. You're likely to
encounter them when they're built lazily. Consider if I added an
imager
attribute to this class:
has 'imager', is => 'ro', lazy_build => 1;
sub _build_imager
{
my $self = shift;
return Imager->new( file => $self->full_file );
}
That performs one very nice feature of decoupling the creation of the Imager
object from the method I want to test. If that's the only value of the new
synthetic attribute, it's still useful. Yet that attribute has greater
serendipities:
sub test_save_to
{
my $module = shift;
my $image = $module->new(
filename => 'my_file.gif',
state => 'FETCH',
validity => 10,
imager => undef,
validity => 0
) );
my $result = $image->save_to( 'some_dir' );
ok ! $result, 'save_to() should return false without valid imager';
ok ! $image->is_valid, '... invalidating image';
}
Extracting this hard-coded call into a synthetic attribute with Moose allows
code which creates these image objects to provide their own values for the
imager synthetic attribute. The test is now in control over what's provided. It
can use a real Imager object or a mocked Imager object. It can allow the lazy
builder to create a real object.
One small act of extraction (and, as usual, intelligent default behavior in
Moose) has turned a method which is difficult to test into something much more
tractable. Testing even the hard-coded call itself was possible in
Perl 5, especially with the utilities of CPAN modules which let you scrounge
around in namespaces, but little bit of care and a little bit of abstraction
and decoupling and encapsulation make the code and the tests more robust.
(For full disclosure: I had a fleeting thought at first "With
imager()
as an accessor, I can subclass the class I want to test
locally, override that method, and return what I want. That's not
strict dependency injection by any means, but I can explain it in
terms of DI as a metaphor." A moment later I realized how easily Moose supports
DI if you structure your code in this way and was glad I don't have to
overdesign the tests.)
This is the sort of writing I didn't have time to get into in detail in Modern Perl: the book, but is well within the scope of what the potential authors of the Moose book want to cover and very much in the spirit of what a new Perl Testing book will discuss. Hint, hint.