Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Conundrum: During the course of working on a new feature or fixing a defect, you find a legacy problem in code. What should you do? Fix it and risk altering the behavior of the code. It has either been working up until now by some fluke, or else the defect has not been detected or worth anyone's time to report. Should you leave it alone and allow the problem to make the code harder to work with later? Fixing the problem will only add to the time of your original task and force you to regression test. Few will appreciate the work. Fixing it, however, seems right somehow. Code with fewer problems is easier to refactor and build upon.

I've been finding myself in this situation time and time again as we work to modernize a web application. I can't tell if I'm being obsessive or honorable when I go off-tangent working on these old bugs. How do you handle these situations?

Thanks, Corey

share|improve this question
add comment

11 Answers

I'll buck the trend here. Unless you are in the very early prototype phase of development, you should never fix it immediately, you should file a bug report. This has several advantages:

  • the team gets to evaluate it. Is it a real bug, what are the risks of fixing it.
  • management gets to decide if is important enough to take the schedule impact of including it in this release
  • a method for detecting it can be added to the test suite, hopefully in a general enough way to also find similar errors.
  • it provides valuable metrics about how many bugs are slipping through the previous phases
share|improve this answer
add comment

Well document the observations first, and decide whether to fix it later.

Have some formal discussion (e.g, in the regular meeting) or informal discussion (e.g., during the lunch) with your colleagues, and make the changes after you gain more confidence on the behavior of the codes you are going to fix.

Although it seems a bug/defect to you, it may actually be a "feature" this time. It could be a poorly implemented solution to get around some corner cases at the last minute of the previous release, and your "clean fix" may revive some previously resolved issues.

share|improve this answer
add comment

As always, it depends.

  • If it's trivial and you are sure you can fix it, fix it.
  • If there are plenty of unit tests, so you can be fairly sure you haven't broken anything, fix it.
  • Otherwise, add a //TODO, add it to your bug tracking, whatever

Basically you're doing a risk assessment: what's the risk of changing vs not changing. If you don't feel like you've got enough experience (with programming in general, or the system in particular) ask someone else in the team.

share|improve this answer
    
+1 - excellent point about existing unit tests. –  Walter Oct 26 '10 at 22:47
add comment

The Pragmatic Programmer calls these 'Broken Windows'.

If you don't fix broken windows then there is a tendency for them to create a downward spiral of code quality. And the more of them there are the bigger job of fixing them, and therefore it is less likely that they will be fixed.

Whether to fix them now or later is up to a matter of judgment. Is it a simple fix? Are you sure the code is doing what you think it is? Is it likely to distract from your current task? Are you under time constraints? Is it likely to introduce more bugs?

At the very least, mark the item in your tracking system and make sure it gets fixed later. It is a important to mark it in the tracking system even if you decide to fix it now, to make sure that it is tested as well, and to document changes.

share|improve this answer
add comment

If the code is obviously wrong, the fix is fairly straightforward and you believe the risk of impacting the users is low, then go for it. It comes down to professional judgement.

You have to remember though that if you've found it then presumably the users haven't, or else they'd have reported it. Rather than spending time fixing an issue that may never be encountered by a user, you might be better off spending that time fixing issues that are causing your users problems now.

share|improve this answer
    
If a user finds a bug, how often are they annoyed at your product and your company, but don't report it to you? I'm guessing a high percentage. –  Craig McQueen Feb 28 '11 at 1:34
add comment

it depends on the bug. the main concern is introducing new bugs. Better to deal with a known issue rather then an unknown one. If its simple, say a text change, or a simple logic error, we fix it, otherwise leave it alone.

One thing to note, tho, we are a small shop of 4 devs and an intern and the bug i fix is probably the bug i created.

share|improve this answer
add comment

I've been on teams where non-critical defects or standard-violations are submitted as a "Weak Code" defect. I would say that the person who finds a critical defect has a responsibility to throw some kind of flag

share|improve this answer
add comment

Our testers hate this. Unless it is very trivial we log it in the bug database, then allocate it to a release and write regression tests. If the developers are just going to making changes that are not on the schedule how can you ever keep a deadline?

share|improve this answer
    
Sometimes making a small bug fix doesn't really take more time than ignoring it, and is more efficient than fixing it later. –  David Thornley Oct 26 '10 at 21:03
    
Thats why I said unless it is trivial. But anything that is going to take more than 15 minutes I think should be logged. –  Craig Oct 26 '10 at 21:54
add comment

I work on a very small team, so it kind of depends on what the change is:

If its a small, obvious bug fix, I definitely go for it. I also throw in extra comments if I have to work through someone else's code and other little improvements that fall under the "boyscout rule" to me.

If the code is so entwined that you have to ask "Will changing this break something and require testing" then no, you shouldn't change it. Bring it up in your bug tracking system if it worries you.

This, incidentally, is why I try to code smaller methods with more obvious type-signatures as well. If you know there aren't side-effects and can make the ins and outs match, you can fix, rearrange, or tweak any of the interior code without risk.

But don't feel like lack of appreciation is a reason not to fix bugs you find or to improve the code base for any reason. If nothing else, you're being kind to the future you who will assuredly be back in there to fix something else.

EDIT: You also need to watch your time on the project. Obviously under tight deadlines, you need to focus on getting the main work done, but if you're just under "normal load" then I think a little cleaning up here and there makes everyone happier in the long run.

share|improve this answer
    
+1 for mentioning the boy scout rule "Leave the campground cleaner than you found it." –  Martin Wickman Oct 26 '10 at 22:04
add comment

It depends, if its a small bug where you are certain that your fix has low impact then personally i would fix it as part of the other work and then let the PM know.

If it has any risk to client, users or company bring it up with the Project Manager and discuss a course forward. Its their job to evaluate risk so bring it to their attention and the case for fixing it. Then honour their decision.

share|improve this answer
add comment

If it's an obvious bug, such as something that will violate security, corrupt data, or raise an exception that gets displayed to the user, then fix it. Otherwise, ask someone who knows the codebase better than you.

share|improve this answer
    
Seems reasonable. What about something that is seemingly minor, like malformed HTML that a browser rendering in Quirks mode tolerates? The bug, in this case, is doing little harm, but I know that it will make life harder down the road if some new content/plugin requires the page to be rendered in standards-compliant mode. –  Corey Oct 26 '10 at 20:57
    
@Corey: Yeah, that's the sort of thing you'd want to consult a more experienced developer on. You have an opinion, and I agree it's probably the right decision, so present your case, but bear in mind that there might be other factors you're not aware of that the guy who's been working on this for 5 years understands. –  Mason Wheeler Oct 26 '10 at 21:19
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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