views:

132

answers:

5

So we have fancy version control systems these days where we can comment on changes made to code. Is it any longer relevant to put similar "documentary" comments in your code? For example, in a file called Helper.cs:

/*
 * Filename: Helper.cs
 * Author: Will Johansson
 * Created: 7/1/2010
 * Purpose: Internal helper functionality for XYZ.
 * Change history:
 *     10/5/2010: Final 1.0 commit. Fixed x, y, z.
 *     9/24/2010: Added functionality Z.
 *     7/1/2010: First version. Added functionalities X and Y.
 */

If not, how would you do it? Leave it to the VCS? Add tags in code like RCS/CVS/SVN for history information? Do it manually? Would you include all the above information, or add more, or remove some?

Additionally, can TFS do something similar to RCS/CVS/SVN as mentioned?

EDIT #1: I would also like to add that I'd like to hear experiences with NDoc and anything else similar.

+2  A: 

It depends on what you want the comments for.

If you're planning to print out the source code, or want the comments close at hand, then add them to the source.

However, I personaly think that's an old fashioned approach. If you may just want to browse the history periodically, then use the VCS - it's easy to enforce the adding of comments with TFS policies, and you can use check in notes to add comments for different teams (we use check in comments for coding technicalities, and then notes for changes that affect UI or documentation, and notes for things testers should specifically look out for). TFS can also link a checkin to a work item for a bug-fix or completed task, which opens up more possibilities for data mining/reports. The final nail in the coffin for source comments is that TFS allows you to browse the history for folders/projects/branches, as well as just individual files.

Jason Williams
Print out source code? I haven't done that since I was a COBOL programmer in the 80's. :)
Robaticus
+1  A: 

Little of the information presented here belongs in comments, in my opinion.

Very little code has a single "author" throughout its lifetime, and it is silly to expect each that each person who touches the code will remember to add their name. Fifteen years and three employers from now, will you want someone calling you to ask questions about this file you created?

"Created" date is also pretty useless information. Who will ever care when something was created?

Change history is completely unnecessary, and is typically incomplete and/or incorrect. A version control system is a far better mechanism for tracking changes. Anyone who is reading the code will probably not care what the code used to look like; they will only care what it is now.

Filename is probably unnecessary. If a person is looking at this file, it is probably because they just double-clicked an icon somewhere that has the file's name, and the file name is probably in the title bar of the editor application. If you print the code as hardcopy, you will probably set a page header or footer that contains the file name. Typically, the file name will not be nearly as important as the names of the classes and functions defined in the file.

A summary of "purpose" makes sense. These are actual code comments, and not administrative information that is better recorded elsewhere.

Copyright and licensing terms may belong at the top of each file, if the source code will be widely distributed. However, in most jurisdictions the copyright is automatically assigned to the author whether a copyright statement is there or not, so there is often no need to protect each individual file.

Kristopher Johnson
+5  A: 

Our coding standards* explicitly state that comments should not provide a historical view of the code. It doesn't matter what the code was, because it was incorrect. That's why it was changed. All that matters is what the code is - the current, authoritative state. It is correct as far as we know (or at least accepted), otherwise we wouldn't have checked it in. Comments should exist to support an understanding of the current state of the code.

This is all in addition to the simple logistical problems that your example poses, of scalability, conflicting and overlapping changes. What happens when classes are completely factored in or out? How is this information contextually useful in a system of any substantive state of change? I can use my own experience as an example here - the platform I lead averages about 30 changes a week, with peaks well over 100, over 3.5 years and likely to extend for at least 3 more. Entire codefiles come and go every day as the system continuously improves. Where does this information go then? How, in a team of ~15 developers, spread across 6 timezones, who come and go over the years, can someone "own" a particular code file? (they can't) And who cares who created the first version?

All this information is stored in a useful, queryable format in parallel with the code in any modern VCS.

*(written by me)

Rex M
A: 

In general, "change history" should be in source control, and having it anywhere else is really asking for trouble. That way you have a brief description of the changes made with each check-in, and you can easily diff versions of the file to get a quick view of the changes. And yes, TFS allows check-in comments, so any time someone makes a change there should be comments explaining what the changeset does.

But I do like the idea of having a small header indicating A) who owns/authored the file, B) what it does/implements, C) the license (if applicable) and D) how to contact the authors of the file.

kidjan
+2  A: 

Get rid of them.

Now! :-)

Comments should add value. Header comments like that don't. They take time to write, clutter the codebase, are treated like banner ads on web pages, quickly get out of date and give no indication as to which lines in the file actually changed.

Instead:

  • Name the class so it's purpose is self explanatory
  • Let the VCS handle history (right click, view history in TFS)
  • Ensure people use check in comments (check in policy in TFS)

P.S. The NDoc question should be asked separately, but for a quick response NDoc is dead - you want to look at using SandCastle instead.

Richard Banks