Comments are not Version Control
In really old legacy code it is common to find comment blocks with complete revision history of each function and old, inactive code that has been commented out. It was fine 20 years ago, when version control systems where not wide spread. Today it’s a code smell.
An old legacy system I used to work with had a comment structure that looked something like the sample below.
// ******************************************************************** // * Logger helper * // * * // * 2005-03-01 First Version, Anders Abel * // * 2007-08-17 Added Console Output, Anders Abel * // * 2009-12-15 Removed file output, John Doe * // * * // * Usage: Call Logger.Write() with string to be logged. * // ******************************************************************** public static class Logger { public static void Write(string message) { //using(StreamWriter writer = new StreamWriter("c:\\temp\\log.txt")) //{ // writer.WriteLine(message); //} Console.WriteLine(message); } } |
The actual code was in C and not C#, but that doesn’t really matter – today’s topic is the comments. There are two things severely wrong with the comments in that piece of code. The number one is the version history. The number two is the commented out code block.
Revision History Belongs in the Version Control
20 years ago it might have been acceptable to not use a version control system. Today it’s not. Even if it’s just a small, personal project, it’s so simple to set up a local repository that there is no excuse to not do so.
With a proper routine for version control in place there is no need to ever have revision history in the source code itself. Those comments belong in the version control commit comments. The version control is also much more fine grained, as it can help show exactly in what revision each line was changed and by whom. The revision comments are far to easy to forget to update.
Never Commit Commented Out Code
The commented out code does not belong in the version control system at all. Version control should only contain the live code, used to run the system.
In my experience, there are two reasons that commented out code has been checked in.
- It’s needed sometimes, e.g. when deploying to the production server but not in the development environment. Trying to handle that manually is a disaster waiting to happen. That kind of environment specific configuration should of course be handled by separate configuration files.
- It’s no longer needed, but might be needed later on. Never do that. Old versions of the code is kept in the version control system and can be brought back if needed.
The Rules
Comments are not a version control system.
Never keep revision history in comments.
Never check in commented out code.
The Exceptions
Every set of rules has some exception. This set of rules is no exception to the rule about every rule having exceptions although the rule about every rule having exceptions say that there might be an exception to the rule itself, creating a rule which indeed has no exception this rule is not that exceptional rule (eehmm… I can hardly parse that myself, let’s go on to the actual exceptions…).
There are some cases where it might be acceptable to deviate a bit from the rules. One example I had myself recently was when I had written a piece of code when developing a work item. When I looked through the final solution I found out a better way to do it, bypassing some helper methods. There was nothing wrong with the helper methods however. I could even see some real use for them later on in the project. In this case I did a commit with the helper methods commented out (they should never be part of a even single build of the system as they are not in use). I then did another commit with the helper methods deleted. That way the helper methods found their way into the version control and can be retrieved later if needed.
Another case is to make the the reader of the code aware of the hidden treasures in the source control history. In this case, a short comment that some functionality has been deleted together with a date is enough to make someone looking at the code later to examine the version control archives.
Some Better Code
/// <summary> /// Logger helper /// </summary> public static class Logger { /// <summary> /// Logs a string ot the log output. /// </summary> /// <param name="message">String to log.</param> public static void Write(string message) { // File logging code removed 2009-12-15. See version control. Console.WriteLine(message); } } |
The comments now follow the standard for the language used. The commented out code block is replaced with a short comment indicating that there is something to be found in the version control, if needed. I’ve found that the projects where version control comments are common often have weak routines for real source control. Whenever you see version info in comments, make sure to check the source control routines (if there are any at all…).
Janus007 on 2012-07-05
I agree…
But I think there’s one more exception to mention, the case when code is under version control, but it is copied around from project to project and thereby loses it history.
In such a scenario it’s very important to keep at least some history comments in the code summary if the changes are major/ crucial so it can be recognized by fellow programmers.
Sounds weird.. yeah :), but that’s the reality.. code is copied around :)
Marnen Laibow-Koser on 2018-05-16
Janus007: Don’t copy (more than tiny bits of) code around without its version history if you can help it. With a VCS such as Git, you can merge one project into another, keeping the history of both parts; also, most modern languages support some notion of libraries and/or packages (with their own version history). Either solution is better than copying code around.