14

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

Corey
  • 241
  • 1
  • 2

11 Answers11

9

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.

CodexArcanum
  • 3,421
  • 21
  • 23
7

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.

Ben Hughes
  • 2,009
  • 1
  • 12
  • 15
5

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.

Dominique McDonnell
  • 1,273
  • 11
  • 14
0

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.

Mason Wheeler
  • 82,151
  • 24
  • 234
  • 309
  • 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
0

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.

Thomas James
  • 697
  • 5
  • 13
0

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?

Craig
  • 4,302
  • 3
  • 21
  • 21
  • 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
0

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

Tim Claason
  • 913
  • 7
  • 11
0

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.

Muad'Dib
  • 2,608
  • 18
  • 14
0

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.

Jason
  • 21
  • 2
  • 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 01:34
0

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.

YYC
  • 506
  • 4
  • 7
0

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
AShelly
  • 5,793
  • 31
  • 51