views:

337

answers:

11

I have to update a legacy application for a critical bug fix. I have to mention upfront that the code i am maintaining is rather bad and numerous mistakes were made that results in performance penalties. The linked-list is implemented rather badly and in some instances incorrectly. The problem goes from bad to worse where core code in the module was cut and pasted to write other components in the system.

Should I re-implement some elements using c++ generics fixing and as much of the good stuff as possible? Or should I keep code change to a minimum? Should I rather choose to patch the code as best I can without re-writing? Either way the work I am performing will be substantial and has a major impact on the system; and this is understood, so full regression testing will be performed.

A: 

Depends on how much time you've got and how much you love that project :)

cherouvim
+13  A: 

Unless you have tests or intend to write tests for the legacy application, it's best to make as few changes as possible, since you will not know what you have broken untill you release the version to the users.

Alex Reitbort
++ That's what http://www.amazon.com/Working-Effectively-Legacy-Robert-Martin/dp/0131177052 suggests as well.
cherouvim
If you don't add tests to the legacy app, and run equivalent tests on your stuff, how will you ever know if you break its replacement?
Chris Ballance
Writing an automated testing tool took a bit of work for about a few hours of unit testing and extra verbose logging. What would have been a rewrite was completed in 4 lines of extra code. The problem unitialised variables fixed with a low risk bug fix instead of new code.
biosFF
+1  A: 

Depends on how much you expect to be w/ the project in the future? If it is your responsibility for the foreseeable future, try and allocate time from mgmt. to improve code.
But don't mix bug-fixing w/ code improvement. It only breeds confusion later on when you can't pinpoint cause of bugs.

Mohit Chakraborty
+4  A: 

From the manager's point of view:

Your effort has a cost associated to it. Can you justify that cost vs future profitability?

From the programmer's point of view:

What the heck, I'd like to fix this up and I'm gonna to do this. Right now.

I guess the best thing is to do a bit of both. Fix as much as you can but not to the point that it becomes bad business.

dirkgently
+1: add unit tests as you go -- that makes it suddenly really important to your manager to improve quality. And makes it possible to make changes.
S.Lott
A: 

Sounds like you know what you are doing to me.

"..full regression testing will be performed."

Just have the tests ready before you start changing anything.

If you split up the linked list in two. It will go twice as fast. :) But you have to know in what part you need to look in. But that can the data answer.

Flinkman
A: 

If you have the resources ( time and people ) to improve the quality of the project without introducing more problems, then I think you should utilize those resources.

My point is that if you don't have the resources to implement and test, you shouldn't make ad-hoc changes to a complex system that you aren't necessarily entirely familiar with.

I wouldn't like being in the position where I make a change to the system and then discover that people in testing don't have time to regression test the changes. Even if you are pretty confident with your changes, they might come back to haunt you after you ship.

Cannonade
A: 

The main thing that would drive me to make a decision one way or the other would be the size of the system.

If it is big and hairy, has a lot of business logic in it, and a lot of users are very used to how it operates, I would lean towards fixing it.

If it is smaller, self contained, doesn't have a ton of business logic, then performance and future maintenance is probably more important than keeping the status quo, so I would lean towards a rewrite.

Guy Starbuck
A: 

Do you want to assume the paternity of the new bugs that come from that fix?

Or even worst, of the underlaying existing bugs that will come up just when you put your hands on the keyboard?

Sometimes you don't have options, but if you do, I would recommend you to just aim and shoot the the single line/piece of code that will fix the problem and run away.

If you don't have the option, and somebody tells you "Hey, congratulations, this is your baby and you'll have to look after him for the next 2 yrs." Then it make sense to start cleaning up the dippers.

OMG, bad memories came to my mind after reading your post. Save your self, you are still on time.... it's a trap, it's a traaaaaaaaaaap

OscarRyz
+2  A: 

From Kernighan And Plauger, ElementsOfProgrammingStyle, 1978

Do not comment bad code, rewrite it.

That being said... I would likely try and ind 3-5 areas that are clearly broken and take that to my boss. Let them make the decision on rewriting or patching it.

If you got here (and it sounds like you did) because the code is not functioning properly then a rewrite is likely needed.

However, speaking from experience (that I wish I had when I did this type of stuff... working with live data for one of the worlds largest banks at 21 can be a little scary!), you need to make sure that you have a clear plan of how you are going to tackle it, and do as much as you can to keep your boss involved in the process. You really need to have people testing it as you go as well. Don't let them try to do the testing at the end. You will likely discover a lot more of things break than you think will as you go.

TofuBeer
So now we know - *you* caused the global financial crisis! You TofuBeer and your untested adolescent programs! :)
MarkJ
I'm way older than that now :-) Fortunately I wasn't the person who "lost" $1,000,000... I also didn't get the promotion after I didn't do it (unlike the person who did).
TofuBeer
A: 

Yes you should fix it. Jeff wrote an awesome post covering this kind of thing (which he refers to as technical debt): http://www.codinghorror.com/blog/archives/001230.html

Leaving this kind of thing untouched is like maintaining debt - and you're already familiar with making the minimum interest payments - in that you have to continue to fix stupid similar bugs.

But it doesn't stop at maintenance time - it also, as Mr. Atwood puts it - "becomes a major disincentive to work on a project." Economic times may be hard right now - but that doesn't mean we should be trying to scare away engineering talent.

A: 

Time is money; but every hour you safe today with a tiny fix will bite you in the long run when it comes to the next problem and this time your tiny fixes breaks everything.

I had issues like this. Issues where I could have solved the problem in a terrible manner within maybe 2 hours. Instead I spent 16 hours refactoring everything. Why the hell have I been doing that? Am I crazy? Simple. The next 4 features I had to add took me each around 30 to 60 minutes. If I had not refactored the code, the next feature had taken me about 4, the second about 6 and the third about 8 hours as the code gets uglier and less maintainable with every feature added. By refactoring into modular, nicely extensible code, adding new features was no work at all any more.

That said, if someone says fix it as fast as possible, I would say "Okay, I will make the ultra fast, ultra hacky fix without refacotring... but only if you promise me that I don't have to ever touch this code again in my whole life thereafter. If you cannot promise that, if you expect me to keep maintaining it, then I refuse to make such a hack".

Remember:
You can tell me

  • what feature set the project must have,
  • within what time frame it must be shippable,
  • to write decent, extensible, maintainable code

Pick any TWO.

Mecki