Decoupling, Testability, and Synthetic Attributes

(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.