views:

194

answers:

8

I have worked a few places which haven’t used source control. They seemed to get into the habit of putting comments around code they changed explaining the change so that things could be reverted.

I have found that this makes the code very difficult to read, and have been fairly adamant that such comments are not needed after introducing source control as the revision history will let you match up tickets with changes.

However now, I am not so sure, I think it may be good to document major revisions to a file in a file as well as in the commit messages. This should make the code more readable. Do people have a best practices way of documenting changes to code, so that it is not too cluttered but is still explanatory to someone trying to read it?

Just to be clear, I am not talking about a list of changes in the head of the file (which is a whole other argument) but comments in the code.

+5  A: 

Most source control systems have an "Annotate" or "blame" command. It shows what code changed with each revision.

Since this information does not change the behavior of the program, or make it easier to understand the program, it doesn't belong in the program.

John Saunders
As I understand blame will just show who changed and which revision each line was changed in. This won't help you for old code, which could have had many revisions.
Jeremy French
A: 

I think it's good to document major changes or refactorings as comments in the code. I agree that it does get a little messy to read around copious amounts of comments, but I think it's easy enough to ignore the comments and try to just read the code. If anything is confusing, or you are tempted to change something, and there is a comment near by, you will be much more inclined to read the comment, as opposed to doing diffs with previous versions to see what had changed and why.

Andy White
+2  A: 

The documentation present in code should describe the code it is near. If the code changes, the documentation should also change accordingly. The version control system should take care of managing what changed and why it changed. Let the code and it's documentation do it's job (Doing things, and describing how/why those things are done), and let th version control system do it's job (controlling/documenting versions and changes).

Whenever you start indicating history in the present(current code), you are asking for trouble. Just think what a particularly change-heavy area of code would look like if it had a number of changes?!

cdeszaq
I don't need to think, I have seen it and it is horrible
Jeremy French
A: 

Don't think that commenting changes inside a code is ever possible in a explanatory way.

But I'd like to say about "Modification history at the file begin". I doubt that this is useful at all. Unless I wanted to know which to versions of a file to compare to see the difference that were introduced with some "NNNN" task. In the file begin there is a line "NNNN date - small description". And our source control can "say" who and in which version this line was introduced.
So without this line I would look through all versions to find correct one.

Mykola Golubyev
+2  A: 

If the comment is time sensitive don't do it it's a bad idea. Put a comment on the source file in the version control system instead.

I see this problem all the time at work. The code base I am currently working on extends back to 1979. You can imagine the difficulty with comments like the one below which have built up over that time:

"Line below seems to fix bug xyz, will revert if unforseen issues occur"

I realise it's not a very descriptive comment in the first place but that sort of thing in svn/cvs/etc. is actually very handy. A comment like that in the code sows the seeds of doubt. Can the comment be removed? Is the unforseen issue that has caused me to have to look back at the code related to this comment? etc.

sipwiz
A: 

Each method in our codebase has its own change history. When the code of a method is changed its header gets a new entry. The code itself does not get cluttered with history comments, only the method header. Each change history entry consists of one or two lines briefly describing the reason for the code change in general terms. The entry also contains a number that refers to a change document, The change document also contains links to bug reports, development projects, design documents etc.

Such entries are invaluable, as they often give you an insight as to why the code is as it is. If you are there to fix a bug the history trace will invariably tell what code change introduced the bug, what the change was attempting to do, and therefore what else you need to be aware of when deciding on a fix.

OTOH - Reams of comments in the body of the code describing what it does and who changed it when are just noise.

lilburne
+2  A: 

I'm just wondering how knowing what the program USED to do helps understanding it now? You say it aids readability. How?

Now, when something is changed because the old way didn't work and the new way is counter-intuitive, I think that should be commented in code, because it's necessary to know for all time "This didn't work, don't do it again. I know it looks like it would make more sense. Still don't do it." Otherwise, I don't see that it helps.

Daniel Straight
A: 

Going to have a stab at answering this myself.

Documenting every change in the source makes it difficult to read, however if there is a particularly counterintuitive piece of code in there and it was introduced for a particular ticket put a note by it as to why the code is strange.

But the code should still be documented. So while you make not put in

/* ticket 101: add blink tag to headers, JF - 20010102 */

You may want to put in

/* make headers blink */

Quite often when people stop doing the former type of comments, they cut down massively on their comments and this is bad.

Jeremy French