Legacy code testing techniques: subclass and override non-virtual members

This post was originally published here.

One of the core techniques in Michael Feathers’ Working Effectively With Legacy Code is the “Subclass and Override Method” technique.  Basically, in the context of a test, we can subclass a dependency and override behavior on a method or property to nullify or alter its behavior as needed.  This is especially useful if we can’t extract a dependency outside of a class under test.  Mocking frameworks such as Rhino.Mocks allow me to verify interactions, or simply put in some canned responses for dependencies.

Since we’re dealing with legacy code, paying down the entire technical debt at once isn’t an option.  We have to make micro-payments by introducing seams into our codebase.  It might make the code slightly uglier at first, but as Feathers notes in his book, sometimes surgery leaves scars.

Some problem code

I need to override a method to provide an alternate, canned value.  However, in C#, members are not virtual by default.  Members have to be explicitly marked “virtual”, otherwise subclassed members can only shadow base members.  For instance, suppose we have class Foo and Bar:

public class Foo : Bar
{
    public decimal CalculateDiscount()
    {
        return CalculateTotal() * 0.1M;
    }
}

public class Bar
{
    public decimal CalculateTotal()
    {
        // Hit database, webservices, etc.
        return PricingDatabase.GetPrice(itemId);
    }
}

I’m trying to test some class that uses Foo.CalculateDiscount method to perform some figure out what shipping methods should be available.  It uses the Foo discount to figure this out.  However, CalculateDiscount calls a base class method (CalculateTotal), which then goes and hits the database!  Not much of a unit test when it hits the database (or web service, or HttpContext, etc.).  Worse, I don’t have access to the Bar class, it’s in another library that we don’t own.  Here’s the test I’m trying to write:

[Test]
public void Should_add_shipping_option_when_discount_is_lower_than_50()
{
    MockRepository mocks = new MockRepository();

    Foo foo = mocks.CreateMock<Foo>();

    using (mocks.Record())
    {
        Expect.Call(foo.CalculateTotal()).Return(100.0M);
    }

    using (mocks.Playback())
    {
        string[] shippingMethods = ShippingService.FindShippingMethods(foo);

        Assert.AreEqual(shippingMethods[0], "OneDay");
    }
}

I could try to remove the dependency on the Foo class somehow.  But there’s a lot of information that my shipping method needs, so I can’t just pass in individual parameters for all of the Foo data.  The Foo class is actually quite large, so trying to extract an interface wouldn’t really help either.  In any case, I just want to override the behavior of the “CalculateTotal” call, and since it’s not marked “virtual”, I can’t override the behavior directly.  For example, this won’t work in my test:

public class FooStub : Foo
{
    public new decimal CalculateTotal()
    {
        return 100.0m;
    }
}

The shipping service knows about “Foo”, not “FooStub”.  Since the FooStub.CalculateTotal is shadowing the Bar method, only when I have a variable of type FooStub will its CalculateTotal get called.  Since the shipping service knows Foo, it will use the Bar CalculateTotal method, even if I pass in an instance of a FooStub.  Shadowing can cause weird things like this to happen, to I try to avoid it as much as possible.

Subclass and override

So subclassing and overriding, one of the main functions of mocking frameworks, won’t work at the test level.  I need to get the override on or between the Foo and Bar classes.  Why don’t we create a new BarSeam class between the Foo and Bar classes?

public class Foo : BarSeam
{
    public decimal CalculateDiscount()
    {
        return CalculateTotal() * 0.1M;
    }
}

public class BarSeam : Bar
{
    public virtual new decimal CalculateTotal()
    {
        return base.CalculateTotal();
    }
}

public class Bar
{
    public decimal CalculateTotal()
    {
        // Hit database, webservices, etc.
        return PricingDatabase.GetPrice(itemId);
    }
}

The BarSeam class now sits between Foo and Bar in the inheritance hierarchy.  BarSeam shadows the Bar.CalculateTotal, with one key addition, the “virtual” keyword.  By making BarSeam.CalculateTotal virtual, I can now subclass Foo and override CalculateTotal, put in a canned response, and not worry about hitting the database.

Yes, BarSeam is ugly, but I want to bring ugly to the front of the class to make sure it doesn’t get swept under the rug.  Since all client code either references Foo or Bar, no client code will be affected as BarSeam’s default implementation merely calls the base method.  Think of it as Adapter, but instead of incompatible interfaces, I’m dealing with a non-extensible interface.

Conclusion

When working with legacy code, it’s not feasible (or even wise) to try and rewrite the application.  By making these micro-refactorings and introducing seams, we’re able to identify places in need of larger refactorings, as well as meeting our original goal of getting our legacy code under test.