views:

334

answers:

18

Why would you add

//Bug 1024

comments into a source controlled code base? Most bug tracking and source control systems are better equipted to keep track of this information. In source control, a label or comment can be used with the checkin. In a bug tracker the revision number can be added to the bug's resolution. So why comment the code? Especial since these comments relevancy is very short lived and they tend to build up littering the code base.

+3  A: 

Pure laziness. Sure, it takes more time in the long run, but in the short run "//Bug 1024" takes no effort whatsoever. The more code you have, the worse this is.

Brian
+4  A: 

Ultimately, I think it's a bad practice. It's better to include why the bug exists (foos of type Y don't have property Z). You can include a "more in BugId 12345" along with that if you want.

If you're integrating on multiple levels, a source code view in trac can link directly to the BugId.

Tom Ritter
+1  A: 

I did this up until Visual Studio 2008 shipped with annotation. It was useful when looking back on old code to immediately see that there at least was thought behind a particular code decision.

Yes, I know you can do compare with previous versions, but that is such a pain in the butt when you just need a quick feel good about minor code updates.

Chris Lively
+3  A: 

Imagine you have a new bug that you track down to the change in revision 12345. You look at the logs and it immediately tells you that Bug 1024 was the reason the change was made.

You can then go and look at 1024 to see what, why and when before making a new fix - the 'one to rule them all'.

If the bug number isn't in the commit message, you have to then search for the bug that a commit fixed - and that might be several (if the bug gets reported more than once).

ColinYounger
+8  A: 

I found one of these helpful the other day in our code base.

I said "why is he calling the initialization function a second time, this late in the workflow??"

The bug comment let me go right the issue description. When I retooled the code, I was sure to include that bug in my tests and didn't reintroduce it.

Although I will say that in general I agree with you and don't insert those myself.

I would have preferred that the developer in question fixed the bug in a more meaningful way, so that I didn't have to wonder about the code in the first place.

rice
This is the one real case where I do such a thing. If there is a particularly nasty bug that changes what you do in a non-obvious fashion, this is fine. Otherwise, avoid.
Ken Wootton
+2  A: 

I would agree with you that the comment like this is not really helpful and is too brief.

It is however extremely useful and important to comment the code with references to the records in the defects tracking system (or to that extend to any KM repository you might have).

Sometimes a code is changed to implement a workaround to a certain issue with the application behaviour. Sometimes the introduced workaround is in no way logical. It often happens that when a code is updated by someone else this 'bad' piece of code is removed as a part of re-factoring effort.

So marking a code as belonging to a particular bug-fix makes it visible during the re-factoring, prompting the developer to review the bug description before changing the code. It also helps in the situation where the bug is re-opened - if you have to change the same part of the code several times you might consider investing time in an alternative solution.

P.S. you might consider this article about MS Office from Joel On Software helpful. As far as I know the code of MS Office and MS Windows is full of similar comments which explain decisions made by the developers long gone.

Ilya Kochetov
+2  A: 

I find it useful when explaining code that would otherwise seem wrong, and also for use in commit messages.

Joel Coehoorn
+1  A: 

If you're browsing through unfamiliar source code, and you see something unobvious, it's nice to know the reason. It's a judgement call though, not every bug fix needs such an explanation - probably most can get away without it.

Mark Ransom
+3  A: 

I think this is a case of "I have a hammer, that must be a nail". Your text editor or IDE is not your only tool for managing the code.

History is best maintained externally to the code. The meaning of the code should be described in comments when not immediately obvious.

I agree that bug numbers should be in source control commit messages.

Darron
+2  A: 

I don't do that. I can't think of a good reason why you would put the defect ID in code. I'll put that on the release notes/changelog instead.

What I do find useful is using the defect Id as part of the name on the automated tests:

[TestFixture]
public class Release_1_2_Bugfixes
{
  [Test]
  public void TestBug42()
  {
    Assert.AreEqual(42, CalculateAnswerToLifeUniverseAndEverything());
  }
}

I've seen other projects doing the same thing.

jop
+1  A: 

If there is enough reason to believe somebody would want to know the bug number when looking at part of the code, adding a comment mentioning the bug can be quite nice (hopefully it will paraphrase the important bits of the bug as well, however).

Yes, source control commit messages should also contain the bug numbers, and looking through the revision logs can give you the same information... but if the same part of the code is changed multiple times, but the details learned from the initial bug still applies, it might take a while to find the original change to ever learn about that bug.

Also, situations arise when there is a good reason to move code from one class to another, or to rename files, which would make it even harder to find the root of the reason behind a certain section of code (renaming not so much of an issue with SVN, but a pain with CVS).

Illandril
+3  A: 

You should never add just the bug number. You should add the bug number and the subject, and any qualifiers if you made multiple checkins for a single bug:

Bug 1111 - Foo busted on 64bit systems. Fix #2 because it was re-opened after the merge to trunk.

Some systems have bug number integration. In mxr.mozilla.org the bug number in the cvs log display is auto-magically turned into a link to the bugzilla.mozilla.org number. When you are digging in the code, this is a huge timesaver. I think that Fogbugz has a similar feature...

Also, even if your system does not, it often helps because nobody wants to see the entire context of the change in comments, that is what the bug tracker is for.

benc
+1  A: 

You hit the nail on the head with "relevancy is very short lived and they tend to build up littering the code base."

Every bit of useless cruft that builds up in source files makes them a little less readable and difficult to maintain. Delete everything that doesn't add value. "Bug 12345" has little value now, and will have none in a few weeks.

Kristopher Johnson
+1  A: 

We work on a large scale system with many developers and multiple released branches.

These bug reference comments can actually be quite useful during porting from one branch to another especially since the SCM system that we use is very feature poor and commit comments are hard to get at or and could be quite old.

If the fix was simple then it may not need a bug marker. If it is non-obvious it may make more sense to refer to a Bug then to write a long explanation in a comment section.

James Dean
A: 

I dislike this sort of graffiti. Like other distasteful life forms they accrete over time, choking the code base.

The trouble really begins when people make bug fixes which overlap a previous bug fix. You then have bug numbers labeling a section of code which are simply wrong or misleading.

DGentry
+2  A: 

I'm surprised at how many people are opposed to this. My personal feeling on this is that these are a very good idea. I agree with an earlier comment that it should include more than just the bug number, and preferably include a short summary and link to the bug tracking system if appropriate.

The benefit to these comments is only obvious in an older project with history and a large number of previous bug fixes. You don't have to make these comments everywhere, but they are very very helpful when placed before a block of code that might not make sense without context. In any kind of reasonably complex system, there will be snippets of code that seem illogical or unnecessary without context.

Due to interactions with the system or old workarounds, the code is necessary. In order to prevent someone later reintroducing a patched bug, it's extremely helpful to denote the bug the code block is designed to fix, preferably with some type of explanation attached. Otherwise you're depending on someone checking the commit history carefully for a reason recorded in the commit log, which is highly unlikely, especially if someone is refactoring code.

EDIT: I'm referring specifically to putting these with a block of code that is unusual or needs the additional context. It's not helpful or necessary to comment every single typo fix you make :-)

Jay
A: 

This type of commenting IS very helpful: what happens when you change bug-tracking or source-control tools? A reference to BZ1722 vs FB3101 would tell you what tracking tool to check (Bugzilla or FogBugz for example).

warren
A: 

It's a good thing!

The person who is looking at the code is unlikely to appreciate the full history of the code and is likely to undo a very important change because they may not have worked in this area of the code before. It can explain code that otherwise looks insane or a customer requirement that is equivalently bizarre.

You cannot always capture the minute particulars of the clients requirements through architecture and code, especially when they ask for something stupid. Hence you start with the sensible and then refine or hack the code into the stupid when you are forced to do so, the bug numbers back up the intent of the crazy code.

Quibblesome