What approaches can I take to lower the odds of introducing new bugs in a complex legacy app?

Question

Where I work I often have to develop (and bug fix) in an old system (.NET 1) whos code is complete spaghetti - with little thought given to variable names, program structure nor comments.

Because of this it takes me ages to understand what bits need changed, and I often 'break' the existing software because I've made a modification. I really really want to spend a couple of months (with colleagues) going through it to refactor but existing developers both can't see the need - nor think theres time for this (the system is massive).

I dread having to work on its code as it takes days to fix something only to find out I've broken something else. This obviously makes me look incompetent - so how can I deal with this?

Answer 1

Start writing tests for the parts you're working on. You can try a workflow that goes something like this:

1. Write tests for the part you're about to change. This will also help you understand how the existing code behaves.
   • Refactor code to support testing if needed, but you may want to write non-unit tests if time is short.
2. Run your tests to make sure things are working as you expect.
3. Make your changes. Refactor if needed in the spirit of continuous code improvement, but don't get carried away.
4. Re-run your tests to make sure you maintained the same functionality.

If you don't throw away your tests, you will over time build up a test suite that should cover most important (and/or volatile) parts of the application and making changes in it will become easier and safer.

You might also find Working Effectively with Legacy Code by Michael Feathers helpful.

Answer 2

I like to follow Uncle Bob Martin's Boy Scout Rule:

"When you have a big messy legacy wad, what you have to do is… What you have to do is stop making the messes and start cleaning them up.

This does not mean you call your managers into a conference room and tell them that you aren’t going to be delivering features for the next three months while you refactor the code. Do NOT do this! Rather, it means that you are going to adopt the “Boy Scout Rule” and check each module in a little cleaner than when you checked it out.

From iteration to iteration, and from release to release, you are going to clean this system while continuing to add new features and functionality to it. There is no other way."

Answer 3

You could explain to the manager that fixes that should take hours end up taking days due to the mess of the code base. The other developers won't see any need for refactoring if they are the original developers - they'll know the system inside out, but management should know there's a risk there if those developers ever leave and take their knowledge with them.

Doing a complete refactoring is not usually feasible, so often you refactor small bits at a time - a couple methods or a module. Heck, if it takes several days to make a fix, maybe you can include a small refactoring of the problematic module a the same time.

Answer 4

Do you really need to spend months refactoring the code? Or could you refactor the code as you make changes. So, for example, if you determine that the Foo method needs to be modified, you could take the opportunity to refactor the Foo method. And if you have to step through a dozen other methods to figure out that Foo is the problem, you can leave behind comments in those methods so that you or someone else in the future knows what the code is supposed to do. Of course, that means that there is still a pile of spaghetti code, but you can at least move the code base in the right direction and make it easier for yourself down the line. Getting multiple months of time to refactor code is going to be a big sell because that means that you're not delivering anything the end user wants during that entire time.

And creating unit tests (or, hopefully, extending the existing test suite) should make it less likely that you inadvertently break something.

Answer 5

Another tip. When the bugs do manifest themselves, and after applying the bandage do not stop there!

Ask the five whys, take the red pill and see how deep the rabbit hole goes, and fix the root cause (and the path down there).

You learn a lot about the system. It helps prioritizing what to fix & refactor. And after a few such trips you have some solid "support beams" to strengthen the monolithic heap of spaghetti.

Answer 6

You can also keep old code in place until absolutely certain that your modifications are sound proof. Just if-case all your changes, so you can toggle them on runtime and quickly pin-point the location of new regression bug:

// old code
...

if (!File.ReadAllText("c:\patch_control.txt").Contains("StrangeUIBugFix=0"))
{
    // new code goes here
    ...
}

// old + new code
int someValue = 
    IsEnabled("StrangeUIBugFix") ? a + b :  // new code
    a * b; // old code

Answer 7

One thing: Never change any existing code if you are not sure what effect change would have on complete application.