Developing Unit Tests While Refactoring Legacy Code?

Question

I have a big code library that I need to refactor in order for it to fit more nicely together (one big problem is there aren't clear enough boundaries between each component). Since there will be some regression testing necessary to validate any refactoring, would it make sense to:

1. Create a unit test for a particular piece of code
2. Run the unit test and make sure it passes
3. Refactor the code and check that the unit test still passes

This seems like a good idea to start using a TDD approach to this refactoring process, although my biggest concern is that I'm writing unit tests after the existing code (instead of writing the code after the unit tests).

Since I'm writing unit tests AFTER the code, should I first refactor, do my own regression tests, then continue using the TDD steps (write failing test, write passing code, refactor, repeat)?

Answer 1

There are 2 books I recommend reading as they will help with moving legacy code to TDD:

Working Effectively with Legacy Code and
Brownfield Application Development in .Net (newer book, but .NET based).

Since I'm writing unit tests AFTER the code, should I first refactor, do my own regression tests, then continue using the TDD steps (write failing test, write passing code, refactor, repeat)?

Write the tests before you refactor. The tests will be more of a specification (the code does X) before you start modifying it. This way, when you refactor the code, the test matches the existing code, so if your refactoring breaks something, it will make existing customers unhappy.

The above 2 books will give you more ideas of how to decide where to break things up into smaller (and more manageable) pieces, and when to leave things alone.

Answer 2

IMO you should go ahead and write the unit tests for passing statements before doing any refactoring. The reason is because you want to be in a known state before making changes.

Answer 3

I think Stacy calls it 'Branching by Abstraction'

For legacy code the best approch I've found is to find pinch points in your code where it's easy to insert an interface covering the inputs and outputs.

Copy your first implementation, refactor, and then have a regression implementation that runs the old way and the new way and stops dead as soon as any difference is found.

Doing this we've managed to replace some things seamlessly that my gut told me however hard I tried we'd land up in trouble, but it worked without a problem.

(* watch out for non-functional differences like performance though!)

Answer 4

You NEED a test to show that the refactoring didn't break anything.

I am a big fan of the A B theory based test that @Squirrel described.

I am also a big fan of characterization tests from Working Effectively with Legacy Code by Michael Feathers.

Personally, this is the technique I use in 90% of my work with legacy code (it's a variation of characterization tests).

1. reduce to functional,
2. run,
3. capture the result with an approval test ( www.approvaltests.com ).

Step 1: Imagine you were refactoring a bunch of functional code. This means; All used parameters are passed in, no side effects, always deterministic (pure); all useful results are returned.

for example, you have a method string Foo(int a, boolean b); this would be easy to test, just throw in some data, and follow steps 2 & 3 and you'd end up with nice characterization test.

Of course this isn't the real world, in the real world I would get something like: void Foo()

So first I would figure out what it's side effects are. They are probably some combination of: 1. change global variables, 2. change database, 3. change files, 4. make web API calls.

so then I would log all those changes:

Log(sql)

Statement.Execute(sql)

Now I can think of the function as

Log Foo()

then run it and see what happens, it probably isn't deterministic, because you haven't passed in any variables, as you start to see what is occurring, you can start to see what needs to be passed in. Let's say it's a database. then you need to effectively do a rollback.

database.startTransaction()

log = LogToString()

Foo()

database.rollback();

Approvals.Approve(log)