Legacy Code Change – a Boy Scout Adds Tests

Here is a legacy code change policy for a team adopting TDD that has a legacy code base:

  • Test-drive new code
  • Add tests to legacy code before modification
  • Test-drive changes to legacy code

Refactoring without tests is dangerous; with all the details we must keep straight, a mistake is easy to make. How many code reviews have you been in where the recommended design changes are not made because “we already tested it”? You avoid the change because it’s dangerous to change code without tests. So, the Boy Scout adds tests too. For more on Boy Scouts, see previous post.

Test-Drive New Code

When you are adding functionality and some or all of it can be implemented in a new function or module, create the code using TDD (In Working Effectively with Legacy Code, Michael Feathers calls this Sprouting.). Then the newly test-driven code can be called as need from the existing legacy code. Michael says “You might not be able to get all those call points under test easily, but at the very least, you can write tests for this new code.” This typifies the Boy Scout approach to always working to make the code a little better than you found it.

Sprouting can be very safe when operations done by the sprouted code do not impact the flow of control of the calling code. When return results are used in conditionals and data structures are modified, then sprouting might not be enough; some behavior preserving tests for the legacy code might be needed also. Where did this policy come from? Good policies come from good principles like the Boy Scout Principle.

Add Tests to Legacy Code Before Modification

Adding tests is easy to say, but not so easy to do when your legacy code was not designed to be tested. Michael wrote a whole book on the topic. You will find yourself in the position of having to make some changes without tests to get the code testable. Do safe refactorings, if there is such a thing. Some reasonably safe changes are adding sense points, or sensing variables. Also it would certainly help to with a partner.

In C, converting a direct call to a call through a function pointer is a pretty safe way to create a test point in otherwise tough to test code. Converting to a function pointer is not so safe when your code is not warning free and some of the callers don’t get to see the function prototype. (Bad things will happen when a direct call is made where an indirect call is what is implemented. I suggest warning free code.)

Don’t feel left out non-C programmers; you can override individual member functions by subclassing your legacy object.

Let’s say your legacy C code already does some printing. We could intercept and capture everything being printed with a spy. Here’s an example of refactoring a direct call to a function pointer, and having a spy infiltrate the legacy code:

//Original prototype from FormatOutput.h
 
int FormatOutput(const char *, ...);
//Original implementation from FormatOutput.c
 
int FormatOutput(const char *, ...)
{
  //snip.
}

Change the signature to be a function pointer, make it extern.

//Modified prototype from FormatOutput.h
 
extern int (*FormatOutput)(const char *, ...);

Rename the implementation, hide the name using static so no code calls it directly. Then initialize the pointer to your implementation.

//Modified implementation from FormatOutput.c
 
static int FormatOutput_Impl(const char * format, ...)
{
    //snip
}
 
int (*FormatOutput)(const char* format, ...) = FormatOutput_Impl;

Now FormatOutput can be overridden during the Setup phase of the test. Here is an example using CppUTest.

TEST_GROUP(ThingBeingTested)
{
    //snip
    void setup()
    {
        UT_PTR_SET(FormatOutput, FormatOutputSpy);
        //snip
    }
 
    void teardown()
    {
        //snip
    }
};

The FormatOutputSpy can capture everything printed, so that tests can inspect what is printed. Notice that Boy Scouts also return function pointers to their original value after each test case runs. In CppUTest, UT_PTR_SET macro overrides a function pointer. During teardown() overridden function pointers are restored.

Test-Drive Changes to Legacy Code

Once you have a few tests in place, the clean up can start. Then you can get to add the new functionality that got you to this code in the first place. The Boy Scout will test-drive in the new functionality.

Don’t feel left out ladies, Girl Scouts add tests too.

Leave a Reply

Your email address will not be published.

Be gone spammers * Time limit is exhausted. Please reload the CAPTCHA.