views:

543

answers:

14

Martin Fowler says that we should do refactoring before adding new features (given that the original program is not well-structured).

So we all want to refactor this dirty codebase, that's for sure. We also know that without unit-testing code it's very easy to introduce subtle bugs.

But it's a large codebase. Adding a complete suite of tests to it seems impracticable.

What would you do in this case?

+13  A: 

My suggestion would be that you touch as little as possible and add what you need. I have found that it is best to leave well enough alone, especially if you are on a tight deadline.

If you had had unit tests, that would be a different story, but when you change code it can be like touching a spider web. Changing one thing can affect everything else.

Kevin
+1  A: 

You have to start somewhere. You can begin by adding tests where you can and build up your test suite as you move along. Even if you have some "silly" tests that do nothing but tell you that you're just able to test some part of the code tells you something.

Nick DeVore
+1  A: 

If adding a suite of tests is impractical, then you have a serious issue with the codebase.

Adding new code and hoping it Just Works is bad.

Write the tests, refactor the base, and add new code. If you can't test it, you'll never know if it's correct.

warren
This is rather naive. The codebase may be huge, old, and a mess but that doesn't mean it doesn't have any kind of testing. If it's in use, and the customer likes it, it's correct.
clintp
+10  A: 

Add unit tests to the code you are going to Refactor.

Mitch Wheat
+1. You can usually create a test to confirm you are not changing the behaviour (even if you don't know whether the current behaviour is what was originally intended.)
finnw
Yes, you absolutely have to do this to prevent regression. Old, ugly code can be hard to understand, preventing you from avoiding any side effects with the change you're making. The downside is, you may end up unearthing and having to fix arcane, hidden bugs withing the existing code!
Ates Goral
Old code often is tightly coupled, making it difficult to add tests. The book Lars A. Brekken recommends directly addresses this and suggests "safeish" ways to decouple.
JeffH
+9  A: 

See Rands in Repose's Trickle Theory for a similar problem he encountered with huge numbers of bugs. His recommendation:

My advice is: START

toolkit
Always a good read
Rob Allen
If adding a test suite is impractical, then don't. Do what you can but don't get paralyzed with fear over something you can't change.
clintp
+3  A: 

Don't think of it as an either/or proposition. There is value added by adding unit tests--you don't have to add an entire suite to get the benefits of adding some tests.

Onorio Catenacci
+5  A: 

When unit tests don't exist or are terribly hard to add with proper coverage in the time frame given, you are going to have to rely on regular testing. In theory, this large and dirty code base you talk about has been deemed fit for use for a time now past. In order to make that determination, there was either programmer testing, QA testing or some kind of testing done by the customer. You want to find out how that was done, what was done, if it's enough to cover what changes you'll make, and then get a commitment to have that done again plus whatever testing is needed for the new code until the product is good enough.

Unit tests are a great service to a programmer, but they're not the only kind of testing out there.

dlamblin
+2  A: 

First thing, add tests for the parts you're going to change. Second, refactor the code until it makes sense to a sane person. Third, make the required changes.

Bill the Lizard
+11  A: 

Let me recommend the book Working effectively with legacy code by Michael Feathers. It contains a lot of realistic examples and shows good techniques for tackling the legacy code beast.

Lars A. Brekken
+2  A: 

A lot of it depends on your language. If you are in a statically typed language, you can probably get away without unit tests. I've worked on lots of jobs that did exactly what you are describing.

If it's a good amount of work (let's say, a man year or 3 people for 4 months) then you probably want to have someone tear apart the code and analyze it first.

If it's a dynamic language, it'll be more problematic--you kinda need some level of unit testing. Perhaps you could add unit tests to areas you need to touch.

I only differentiate between static and dynamic languages because it's so much easier to do refactorings in a statically typed language--they tend to be a lot more predictable. I'm not hating Ruby or anything--I spent a year on ROR as well. I just think they need different approaches.

Bill K
+3  A: 

I find myself in this situation quite frequently, as that's pretty much what I've been stuck with for the last two years.

The right approach really depends on social and organizational aspects more than on the technical side of things. Your job is to generate value for your organization. If refactoring will generate more value than it costs, then you should be able to sell it. In my case, the key factors include:

  1. Expected ownership of the project in question. If you expect to be a significant stakeholder in this particular piece of software for the forseeable future, that's an argument in favor of making more extensive modifications to a bad code base b/c it'll pay off more as you maintain it going forward. If you're adding a drive-by feature, adopt a more hands-off approach.

  2. Complexity of the changes being made. If you're making deeply complex changes to the codebase (a typical case in a "dirty" codebase, b/c such source is generally tightly coupled and incohesive), some refactoring is more likely to be called for. Such changes aren't the result of being a code ninja, either, as they're necessary for you to simply reason about the changes that you're making. This is also related to the "badness" of the codebase you're modifying. It's practically impossible to create even the simplest unit tests for a tightly coupled, incohesive mess. (I speak from experience. One of the projects I almost got stuck maintaining was about 20k lines long, excluding generated code, in twelve files. The entire app was a single class called "Form1". This is an example of a dev abusing the partial class feature.)

  3. Organizational oversight. The strength and strictness of your organization's oversight comes into play here. If your group actually performs some core best practices, such as code reviews, and doesn't just pay lip service to them, I'd be more inclined to not make extensive refactorings. The value tradeoff is probably weighed more in favor of touching as little as possible because you've got another pair of fresh eyes checking to make sure that none of the few changes you did make have undesirable side effects. Similarly, stricter oversight is more likely to frown on the "guerilla" code tactics of making changes that aren't strictly called for in the change request.

  4. Your boss. If your boss is on your side, you're far more likely to be able to make long-term value-improvement on your codebase, especially if you can justify the increased cost now in terms of budgetary hours later down the road. Remember that your manager has a better perspective on this software's role in the big picture than you do. If it's a piece of software that's only used by ten or twenty people, it just doesn't call for the sort of long-term maintenance improvements that a piece of software used by ten or twenty thousand people calls for.

The core question you need to answer when considering any sort of time investment like this is, "Where does the value lie?" Then you need to chase that value.

Greg D
+2  A: 

Fowler also suggests that you never refactor without the safety of tests. But, how do you get those tests in place? And, how far do you go?

The previously recommended book (Working Effectively with Legacy Code by Michael Feathers) is the definitive work on the subject.

Need a quicker read? Take a look at Michael's earlier article (PDF) of the same name.

Alan Ridlehoover
I've already borrowed the book from a library. It's a good read. Thank you guys!
Owen
+1 for Fowler's suggestion of not refactoring without tests.
JeffH
A: 

touch as little as possible. And pray.

Kevin
A: 

I agree with the others, touch as little as possible. But, also, you can always start writing unit tests for new functionality and/or existing functionality that you'll be touching. No one said you need to write unit tests to test 100% of the existing code base to start with.

Chris Pietschmann