views:

273

answers:

6

Let me pose a bit of background information before asking my question:

I recently joined a new software development group that uses Rational tools for configuration management, including a source control and change management system. In addition to these tools, the team has a standard practice of noting any code changes as a comment in the code, such as:

///<history>
   [mt] 3/15/2009  Made abc changes to fix xyz
///</history>

Their official purpose for the commenting standard is that "the comments provide traceability from requirement to code modification".

I am preparing to pose an argument that this practice is unnecessary and redundant; that the team should get rid of this standard immediately.

To wit - the change management system is the place to build traceability from requirement to code modification, and source control can provide detailed history of changes by performing a Diff between versions. When source code is checked in, the corresponding change management ticket is noted. When a CM ticket is resolved, we note which source code files were modified. I believe this provides a sufficient cross-reference for the desired traceability.

I would like to know if anyone disagrees with my argument. Am I missing some benefit of commented source code history that change management and source control systems cannot provide?

+4  A: 

A comment allows you to find all the changes and their reasons in the code right where they are relevant without having to dig into diffs and version control system intricacies. Furthermore, should you decide to change of version control system, the comments will stay.

I worked on a large project with similar practice that had changed of source control system twice. There wasn't a day when I wasn't glad to have these comments.

Is it redundant? Yes. Is it unnecessary? No.

Julian Aubourg
You can use annotate tool available in any sane VCS.
Eugene Morozov
Right until you change of version control system and loose said annotations.
Julian Aubourg
sort out the *real* problem, e.g inport all the change control comments from the old version control system into the new version control system. If the new system can not do this, why did you buy the new system?
Ian Ringrose
+1  A: 

The one that jumps to mind to me is vendor lockin. If you ever moved away from Rational, you'd need to make sure that the full change history was maintained during the migration - not just the version of the artifacts.

JohnW
Isn't this a bit of an issue anyway? At least, if you plan to support any old versions of your software.I believe it is more important to properly document what the current version of the code is doing in the source. If I want to see what it used to do, that's where it seems source control steps in
mtazva
+1  A: 

When you're in the code you need to know why it's structured like that, hence in code commenting. Tools that sit outside the code, good though they may be, require far too much of a context shift in your brain to be useful. As well as that, trying to reverse engineer the code intent from documentation and a diff is pretty damn hard, I'd much rather read a line of comment any day.

MrTelly
To be clear - I'm not talking about comments related to the current code set. I'm talking about comments that note the change history. I agree that any code that is not sufficiently "self-documenting" should have helpful comments. My concern is having the entire history of a function in the source.
mtazva
+5  A: 

For myself, I have always found such comments to be more trouble than they're worth: they can cause merge conflicts, can appear as 'false positives' when you're trying to isolate the diffs between two versions, and may reference code changes that have since been obsoleted by later changes.

It's often (not always, but often) possible to change version-control systems without losing metadata. If you were to move your code to a system that doesn't support this, it would not be hard to write a script to convert the change history into comments before the cutover.

Dan Breslau
"may reference code changes that have since been obsoleted by later changes" this by far is the most annoying. I don't get why people add comments when they add something, but don't think to do the same when they remove something!
matt b
This is my primary concern, in addition to the bloat caused by a long history of changes to a function. I believe source code should only document it's CURRENT state. All other documentation belongs somewhere else (silence the noise!) BTW My favorite cmt so far is:[ab] 1/1/2005 Created function
mtazva
A: 

There was a phase in the code I work on, back in the 1994-96 time frame, where there was a tendency to insert change history comments at the top of the file. Those comments are now meaningless and useless, and one of the many standard cleanups I perform when editing files containing such comments is to remove them.

In contrast, there are also some comments with a bug number at the location where the change is made, typically explaining why the ridiculous code is as it is. These can be very helpful. The bug number gives you somewhere else to look for information, and fingers the culprit (or victim - it varies).

On the other hand, items like this one - genuine; cleaned up last week - make me grit my teeth.

    if (ctab->tarray && ctab->tarray[i])
#ifndef NT
      prt_theargs(*(ctab->tarray[i]));
#else
      /* Correct the parameter type mismatch in the line above */
      prt_theargs(ctab->tarray[i]);
#endif /* NT */

The NT team got the call correct; why they thought it was a platform-specific fix is beyond me. Of course, if the code had used prototypes instead of just parameterless declarations before now, then the Unix team would have had to fix the code too. The comment was a help - assuring me that the bug was genuine - but exasperating.

Jonathan Leffler
+2  A: 

I've always thought that code should be, of course, under version control, and that the current source code (the one that you can open and read today) should be valid only in present tense.

It doesn't matter if a report could have up to 3 axis in the past and last month you updated it to support up to 6 axis. It doesn't matter if you expanded some function or fixed some bug, as long as the current version can be easily understood. When you fix a bug, just leave the fixed code.

There's an exception, though. If (and only if) the fixed code looks less intuitive to you than the previous, incorrect one; if you feel that someone might come tomorrow and, just by reading the code, be tempted to change it back to what "seems more correct", then it's good to add a comment: "This is done this way to avoid... blah blah blah." Also, if the problem behind is an infamous war story inside the team's culture, or if for some reason the bug report database contains very interesting information about this part of the code, I wouldn't find it incorrect to add "(see Bug Id 10005)" to the explaining comment.

Daniel Daranas
I whole-heartedly agree, Daniel. The "this is done this way because..." qualifies as a standard code-documenting comment, rather than a historic comment, and is absolutely necessary. When I open source code to make changes, I care about what the code does, not what it used to do.
mtazva