Code maintenance: To add comments in code or to just leave it to the version control?

We have been asked to add comments with start tags, end tags, description, solution etc for each change that we make to the code as part of fixing a bug / implementing a CR.

My concern is, does this provide any added value? As it is, we have all the details in the Version control history, which will help us to track each and every change?

But my leads are insisting on having the comments as a “good” programming practice. One of their argument is when a CR has to be de-scoped/changed, it would be cumbersome if comments are not there.

Considering that the changes would be largely in between code, would it really help to add comments for each and every change we make? Shouldn’t we leave it to the version control?

You are absolutely right. Tracking changes is the job for your version control system. Every time you do a commit you should write a commit message explaining what was done, and referencing your bug-tracking system if this is a bug fix. Putting a comment in the code saying

// begin fix for bug XXXXX on 10/9/2012
...
// end fix for bug XXXXX

every time you fix a bug will quickly render your code unreadable and unmaintainable. It will also result in duplicating the same information in two places, which will make the mess even worse.

Comments should not be used for bug tracking and they also should not describe what your code is doing. They should explain why you are doing X, or why you are doing X in this particular way. If you feel the need to write a comment explaining what a block of code is doing, that is a code smell which indicates that you should refactor this block into a function with a descriptive name.

So instead of

// fixed bug XXXXX on 10/9/2012

you might have a comment that says

// doing X, because otherwise Y will break.

// doing X, because doing Y is 10 times slower.

Use the best tool for the job. Your version control system should be the best tool for recording when bugfixes and CRs are made: it automatically records the date and who made the change; it never forgets to add a message (if you’ve configured it to require commit messages); it never annotates the wrong line of code or accidentally deletes a comment. And if your version control system is already doing a better job than your comments, it’s silly to duplicate work by adding comments.

Readability of source code is paramount. A codebase that’s cluttered with comments giving the full history of every bugfix and CR made is going to not be very readable at all.

But don’t skip comments completely: Good comments (not slavishly documenting every start / stop / description / solution of every bugfix and CR) enhance the readability of the code. For example, for a tricky or unclear bit of code that you add to fix a bug, a comment of the form // fix ISSUE#413 telling people where to find more information in your issue tracker is an excellent idea.

Comments in the code are about what the code is at that moment. Taking a snapshot at any given time shouldn’t refer to old (or worse, future) versions of the code.

Comments in VCS are about how the code has changed. They should read as a story about development.

Now, every change should include comments? in most cases, yes. The only exception I imagine is when the expected behaviour was already documented but wasn’t what you got, because of a bug. Fixing it makes the existing comments more precise, so they don’t have to be changed. The bug itself should be documented in the ticket history and the commit comment, but only in the code if the code looks strange. In that case, a // make sure <bad thing> doesn't happen should be enough.

One type of comment I really appreciate is:

// This implemented for Business Rule 5 of Proposal 2

or whatever the heck you use to gather your requirements.

This has two advantages, one is that it lets you find the reason you implemented a given algorithim without searching and another is that it will help you communicate with non-software engineers that work on/create the requirements docs.

This may not help with smaller teams but if you have analysists that develop your requirements, it can be invaluable.

Your leads are right when they say that comments are a good programming practice, however there are exceptions. Adding a comment for every change that you make is one of them. And you are right by saying that this should belong to the version control system. If you have to keep these comments in a single place, then the VCS is the way to go. Comments in source code tend to grow old and unmaintained. No comments are a lot better than bad comments. What you don’t want is having comments in both places (in the code and VCS) that are out of sync. The goal is to keep things DRY by having a single source of truth for changes to the code.

In addition to what others have said, consider what happens if a change has ripple effects throughout the system. Say you refactor a part of a core interface in the process of implementing a change request – that kind of change can easily touch a large percentage of the source code files in any non-trivial piece of software with what amounts to trivial changes (class or method name changes). Are you supposed to go through every single file touched by such an operation to manually annotate it with such comments, rather than relying on the VCS doing it all automatically? In one case you’re looking at little more than a five minute job with any decent refactoring tool followed by a recompile to make sure nothing broke the build, whereas the other can easily balloon into a day’s work. For what specific benefit?

Also consider what happens when you move parts of the code around. One of the database developers I work with is in the camp of largely “each line of SQL should be annotated with the revision in which it was changed, and we’re going to do separate revision histories for each file because then it’s easier to see who changed what when and why”. That works kinda-sorta okay when the change is on the order of changing single lines. It doesn’t work quite as well when, like I did recently to fix a serious performance issue, you break out parts of a larger query introducing temporary tables, then change the ordering of some of the queries to better fit the new code flow. Granted, the diff against the previous version was largely meaningless since it said about two thirds of the file had changed, but the check-in comment was also something like “major reorganization to fix performance issues”. By the time you looked manually at the two versions, it was pretty clear that large parts really were the same, only moved around. (And it took the stored procedure in question from regularly taking over half a minute to execute, to a few seconds. By that time, it was largely I/O-bound with few places for significant further improvement at least in the short term.)

With very few exceptions, change tracking and issue referencing is the work of the VCS, IMNSHO.

I usually follow this rule: if the change is obvious and the resulting code does not raise questions, does not revert or substantially change any previous behavior in a substantial way – then leave it to the VCS to track bug numbers and other change information.

However, if there is the change that is not obvious, that changes the logic – especially substantially changes the logic done by somebody else in a non-obvious way – it may be very beneficial to add something like “this change is to do this and that because of bug #42742”. This way when somebody looks at the code and wonders “why is this here? this looks weird” he has some guidance right in fron of him and does not have to do investigation via VCS. This also prevents situations where people break other’s changes because they are familiar with old state of the code but do not notice it has been changed since.

Version-control-related comments do not belong into the source file. They only add clutter. Since they are likely required to be put in the same place (like a comment block at the top of the file) they will cause comment-only nuisance conflicts when parallel branches are merged.

Any tracking information that can be pulled out of version control should not be duplicated in the body of the code. That goes for silly ideas like the RCS checkout keywords like $Log$ and its ilk.

If the code ever travels outside of the scope of the version control system, that trail of comments about its history loses context and therefore most of its value. To properly understand the description of the change, we need access to the revision, so we can view the diff to the previous version.

Some old files in the Linux kernel have big history comment blocks. Those date back to when there was no version control system, just tarballs and patches.

Comments in the code should be minimal and accurate. Adding defect /change information not valuable. You should use version control for it. Some time Version control provides a slightly better way of change- We use ClearCase UCM ; UCM activities are created based on defect numbers, change area etc ( e.g. defect29844_change_sql_to_handle_null).

Detailed comments is preferred in check-in comments.

I prefer to include provide details about back ground information, details of solution NOT implemented due some side effects.

Pramagic Programmer and CleanCode leads to following guideline

Keep the low-level knowledge in the code, where it belongs, and reserve the comments for other, high-level explanations.

Filed under: softwareengineering - @ 19:20

Thẻ: bug, change, comments, version-control

Thiết kế website giá rẻ

Danh mục

Code maintenance: To add comments in code or to just leave it to the version control?