50

One of the most unpleasant (and unfortuantely most frequent) situations I am confronted with in my all day life as a developer is that I have to fix bugs or add features into code that is badly designed. Now as a good craftsman I would like to leave the code in a better state than I found it. Often new features can not be implemented if I do not refactor the design. Well - they could, but that would make the code even worse.

Unfortunately this is exactly what I tend to have a hard time with. I feel that if there is one thing that is hard, it is to refactor bad code, especially when you have deadlines. Touching bad and complex code that more or less works is scary. As a result I introduce even more clutter when I hack a new feature into the code without modifiying existing code.

Now my question is How can I learn to cope with bad code? How can I learn to understand huge codebases and then refactor parts of it without breaking stuff that already worked and without exceeding the deadline? Is there any literature you can recommend? Do you have any general tips for me?

0

9 Answers 9

24

General tip:

if (it is working)
   Do (not touch it);
else
{
   Make (as few modifications as possible)
   Or (it will stop working at all);
}

This is experience of generations.

8
  • 12
    I agree, but doesn't it frustrate you to see how bad it's done? It's like I HAVE TO FIX THIS!
    – Strawberry
    Jan 20, 2010 at 8:47
  • 3
    Every programmer has that ;) but time, and thus money, wont allow for it.
    – Oxymoron
    Jan 20, 2010 at 8:48
  • 3
    This will introduce even more ugly hacks and workarounds to kill a bug or to introduce a new feature. If always I go the route of least resistance after years the code becomes an untouchable black box (i.e. black hole)
    – bitbonk
    Jan 20, 2010 at 8:56
  • 2
    @bitbonk: that's still more commercially viable than the netscape route: joelonsoftware.com/articles/fog0000000069.html Jan 20, 2010 at 9:01
  • 4
    …This is why the Big Ball of Mud keeps getting worse and worse.
    – Jon Reid
    Jan 23, 2010 at 18:51
22

Michael Feathers wrote a good book about this subject exactly.

Working Effectively with Legacy Code.

Another great book is by Martin Fowler, Kent Beck and others:

Refactoring: Improving the Design of Existing Code.

1
  • 2
    The books are a pair. I recommend reading Refactoring first. It will inspire and frustrate, because you'll say, "But I need to make changes to make it testable. How do I do that without breaking the code?" That's where the Feathers book comes in.
    – Jon Reid
    Jan 23, 2010 at 18:50
11

Refactoring needs the safety harness of a unit test suite to remove that "Have I broken it?" feeling. Covering the bad code in a blanket of tests will help you whilst you strive for good clean code.

Pex is a tool that I find useful (if you are in the .NET world) for creating tests for legacy code.

Legacy code == code without tests!

8

When I have to deal with adding functionality to bad code, my usual approach is:

  • Write automated tests for every important feature that must work (as most bad code doesn't have any tests).
  • Do code changes.
  • Make sure the tests are still working.

That give you at least some confidence that you didn't break everything. As for how to learn coping with bad code, I guess it's just about experience.

3
  • 10
    Very often code is so badly designed it is hard to write tests for it at all.
    – bitbonk
    Jan 20, 2010 at 8:50
  • It is always possible to write at least some tests. Jan 20, 2010 at 8:51
  • 2
    You can at least write tests against the highest level interfaces. If there are no good interfaces, add a wrapper which gives a good interface, then write the tests against that.
    – Ether
    Jan 20, 2010 at 18:40
5

Well, if you're going to refactor large amounts of code in a project I'd recommend using some decent version control, so you can branch and fall back easily. Given, this is probably is an open door, but crucial imo.

Furthermore, before you start getting in to complex OO, try breaking methods and functions into smaller ones. Ensuring some level of atomicity in each of the functions, which makes the code alot easier to maintain, read and manage. It's all about the small stuff, break it down into logical units of operation, I'm doing a refactor action on a 1k lines method. It does al sorts of fancy stuff. My first goal is to get as much stuff out of it in smaller pieces, when that's done I'll start thinking about a better OO design, which is much easier because I've a much better grasp on things.

Aspirin works good too.

4

I am currently in this situation. My approach is to answer some questions before touching the code:

  1. Is the code really that bad? If yes, what are the common errors? ==> maybe concentrate on those first
  2. What is the main runtime flow in the code? Maybe you can discard quite a few constructs from it.
  3. Try to layer/modularize the code without changing it. This leads to some reduction of interdependencies
  4. Try to poke the code with tests. If the code base is entangled beyond hope: use something like PowerMock to simulate objects that are not (yet) in need of change
  5. Have an integration environment available, in which you can test changes in a production near environment.
  6. Don't shy away to rewrite parts of the code base. But try to not implement too much new stuff in it
  7. Try to team up, discuss designs, principles, solutions

This is tough work, and nobody will thank you for it. Be proud of small improvements and enjoy a good work done :)

1

I think that it is always good to have a general idea of how everything works in the software you are developing/improving. That's where the design documents and other documents made after or during the development process come in. I believe that if someone before you has not done proper documentation, then at least you should write a couple of lines somewhere about what you experience throughout your development process. I usually use OneNote or other stuff to write notes about what I encounter, and usually keep listing things which I feel would require refactoring. And if there is some downtime during the project, I usually go back to that list and try to improve things bit by bit.

So basically, if someone before you hasn't done it right, it would be good if at least you could help reduce the problems for any other developer who would come across the same code.

0

It depends on number of factors but the most important is if you have authority to modify it.

In case you do, refactor it. For example, rename classes/functions/variables. Extract and generalize functionalities. See Refactoring: Improving the Design of Existing Code (Bible for the subject). Before you begin doing that ensure that code is in a proper version control (VC) and have a good set of test cases. VC let you roll back and test cases help catching unexpected side-effects.

I suggest Distributed Version Control like Mercurial/Bazaar and Git because it is very refactoring is not exactly structured like adding features.

If there were no tests (common), you must create them. Read Working Effectively With Legacy Code. Especially about "Seal point" (not about Siamese cat :p).

In case you don't create a wrapper API that is cleaner.

For example:


Old code ====================
const ACT_SHOW = 'show';
const ACT_HIDE = 'hide';
function int DoThing(Object $Obj, Stirng $Action, Object $Param1, Object $Param1) {
     ....;
}
Added code ==================
enum Actions {
    show, hide;
};
class ActionDoer {
    private obj;
    ActionDoer($Object) {
        this.obj = $Object;
    }
    function int act(Actions $Action, $Param1, $Param1) {
        this.act($Action.toString(), $Param1, $Param1) ;
    }
    function int act(String $Action, $Param1, $Param1) {
        DoThing(this.obj, $Action, $Param1, $Param1) ;
    }
    function int show() {
        this.act(Actions.show, null, null);
    }
    function int hide(Color $ToBGColor, long $FadeTime) {
        this.act(Actions.hide, $ToBGColor, $FadeTime);
    }
}

This way, the old code is not touched and the extension can be done using the new code. Good example of this method is jQuery where the old (default) way of accessing DOM is painful.

Hope this helps.

Not the answer you're looking for? Browse other questions tagged or ask your own question.