Don’t comment bad code - Rewrite it.
Nothing can be quite so helpful as a well placed comment. Nothing can clutter up a module quite like bad comments.
Comments are not pure good - They are, at best, a necessary evil. The proper use of comments is to compensate for our failure to express ourselves in code. When you find yourself in a position where you need to write a comment, think it through and see whether there isn’t some way to turn the table and express yourself in code.
One major problems with comments is that they lie, not always and not intentionally but too often. The reason is simple, programmers can’t realistically maintain them. Code changes and evolve, unfortunately the comments can’t always follow them. For example this code:
MockRequest request;
private final String HTTP_DATE_REGEXP =
“[SMTWF][a-z]{2}\\,\\s[0-9]{2}\\s[JFMASOND][a-z]{2}\\s”+
“[0-9]{4}\\s[0-9]{2}\\:[0-9]{2}\\:[0-9]{2}\\sGMT”;
private Response response;
private FitNesseContext context;
private FileResponder responder;
private Locale saveLocale;
**// Example: ”Tue, 02 Apr 2003 22:18:49 GMT”**
Other instances variables were probably added between the HTTP_DATE_REGEXP constant and its explanatory comment.
Inaccurate comments are far worse than no comments at all. They delude and mislead, they set expectation that will never be fulfilled.
Truth can only be found in one place: the code.
Comments don’t make up for bad code
One of the more common motivation for writing comments is bad code - Instead of commenting this you better clean the code!
Clean and expressive code is far superior to cluttered and complex code with a lot of comments.
Explain yourself in code
There are certainly times when code makes a poor vehicle for explanation. But this is often false, would you rather see:
// Check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) &&
(employee.age > 65))
-or-
if (employee.isEligibleForFullBenefits())
It takes only a few seconds of thought to explain most of your intent in code.
Good comments
Some comments are necessary and beneficial
Legal comments
Sometimes corporate coding standards can force us to write comments for legal reason, for example copyright and authorship statement.
Informative comments
It is sometimes useful to provide basic information with a comment- Given this code for example:
// format matched kk:mm:ss EEE, MMM dd, yyyy
Pattern timeMatcher = Pattern.compile(
“\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*”);
In this case the comment lets us know that the regex is intended to match a time and date that were already formatted - Even if this class could have been made better the comment here is useful.
Explanation of intent
Sometimes a comment can go beyond just useful information - It can provide an intent behind a decision. While you may not agree with the programmer’s solution to the problem you at least knows what he was trying to do.
**//This is our best attempt to get a race condition
//by creating large number of threads.**
for (int i = 0; i < 25000; i++) {
WidgetBuilderThread widgetBuilderThread =
new WidgetBuilderThread(widgetBuilder, text, parent, failFlag);
Thread thread = new Thread(widgetBuilderThread);
thread.start();
}
Clarification
Sometimes it is just helpful to translate the meaning of an obscure argument or return values - In general it is better to find a way to make that argument/value clearer in its own but sometimes it is part of a standard library or codes that we do not own. The risk here is that the clarifying comment is incorrect!
Warning of consequences
Sometimes it is useful to warn other programmers about certain consequences. For example we could comment out a test that takes too long with a warning
# Dont uncomment this unless you've got time to kill
def long_test(value):
...
There are other ways of doing it, for example the pytest.skip mark for Python but the comment works.
TODO Comments
This is something I, personally, struggle a lot with - In my opinion we should never ever leave a to do comments, this is as good as marking ‘Will never do it but I don’t want to admit it yet’. The author however points out that there is sometimes use for TODO comments, it might be a request for an other programmers to delete a deprecated feature or to find a better name. However leaving a TODO is not an excuse for leaving bad code in the system - The author also advice to scan through the to-dos regularly to eliminates the one you can.
Amplification
The comment can be left to amplify the importance of something that may otherwise seems inconsequential
String listItemContent = match.group(3).trim();
**// the trim is real important. It removes the starting
// spaces that could cause the item to be recognized
// as another list.**
new ListItemWidget(this, listItemContent, this.level + 1);
return buildList(text.substring(match.end()));
Javadocs in public APIs
If you are writing a public API, then you should certainly write good javadocs for it. But keep in mind the rest of the advice in this chapter. Javadocs can be just as misleading, nonlocal, and dishonest as any other kind of comment.
Bad comments
Most comments fall into this category. Usually they are crutches or excuses for poor code amounting to little more than the programmer talking to himself.
Mumbling
Plopping in a comment because you felt you should is a hack. Comments should be well though of and be useful. For example:
public void loadProperties()
{
try
{
String propertiesPath = propertiesLocation + ”/” + PROPERTIES_FILE;
FileInputStream propertiesStream = new FileInputStream(propertiesPath);
loadedProperties.load(propertiesStream);
}
catch(IOException e)
{
**// No properties files means all defaults are loaded**
}
}
It means something for the author but does not come through all that well. Our only recourse here is to examine the code to find out what’s going on
Redundant Comments
Sometimes comment may just repeat what the code is doing with no/little added value - Take for example:
**// Utility method that returns when this.closed is true. Throws an exception
// if the timeout is reached**.
public synchronized void waitForClose(final long timeoutMillis)
throws Exception
{
if(!closed)
{
wait(timeoutMillis);
if(!closed)
throw new Exception("MockResponseSender could not be closed");
}
}
```
This comments tell us what the code already is saying clearly
It does not justify the code or provide intent.
##### Misleading comment
Sometimes, even with the best intention, a programmer makes a statement in his comments that isn't precise enough to be accurate, lets take for example this comment:
// Utility method that returns when this.closed is true. Throws an exception
// if the timeout is reached.
public synchronized void waitForClose(final long timeoutMillis)
throws Exception
{
if(!closed)
{
wait(timeoutMillis);
if(!closed)
throw new Exception(“MockResponseSender could not be closed”);
}
}
Here the comment tells us it return when this.closed becomes true - However when looking at the code you can see that it actually returns only if this.closed is true otherwise it waits for a blind time out and throw an exception if this.closed is still not true.
#### Mandated Comments
It is just plain silly to have a rule that says every function must have a javadoc/comment. Requiring this leads to comments such as:
/**
*
* @param title The title of the CD
* @param author The author of the CD
* @param tracks The number of tracks on the CD
* @param durationInMinutes The duration of the CD in minutes
*/
public void addCD(String title, String author,
int tracks, int durationInMinutes) {
CD cd = new CD();
cd.title = title;
cd.author = author;
cd.tracks = tracks;
cd.duration = duration;
cdList.add(cd);
}
#### Journal Comments
Sometimes people add a comment to the start of every module every time they change something like:
- Changes (from 11-Oct-2001)
* --------------------------
* 11-Oct-2001 : Re-organised the class and moved it to new package
* com.jrefinery.date (DG);
* 05-Nov-2001 : Added a getDescription() method, and eliminated NotableDate
* class (DG);
* 12-Nov-2001 : IBD requires setDescription() method, now that NotableDate
* class is gone (DG); Changed getPreviousDayOfWeek(),
* getFollowingDayOfWeek() and getNearestDayOfWeek() to correct
* bugs (DG);
These comments accumulate as a kind of journal/log which would have been useful years ago when source code control system weren't a thing.
#### Noise Comments
Sometimes you see comments that are nothing but noise.
/** The day of the month. */
private int dayOfMonth;
Those comments are so noisy that we learn to ignore them, by ignoring them, eventually they will become lies.
Another type of noisy comments are venting:
private void startSending()
{
try
{
doSending();
}
catch(SocketException e)
{
// normal. someone stopped the request.
}
catch(Exception e)
{
try
{
response.add(ErrorResponder.makeExceptionString(e));
response.closeAll();
}
catch(Exception e1)
{
//Give me a break!
}
}
}
### Scary noise
Javadocs can be noisy, what purpose do the following serve:
/** The name. */
private String name;
/** The version. */
private String version;
/** The licenceName. */
private String licenceName;
/** The version. */
private String info;
Notice that there is a copy-paste error in said doc, but your brain doesn't read all this anyway.
### Don't use a comment when you can use a function or variable
Sometimes comment should be an indicator that we can refactor something to make it clearer, given the following:
// does the module from the global list
// subsystem we are part of?
if (smodule.getDependSubsystems().contains(subSysMod.getSubSystem()))
We can rephrase all that, without the comment:
ArrayList moduleDependees = smodule.getDependSubsystems();
String ourSubSystem = subSysMod.getSubSystem();
if (moduleDependees.contains(ourSubSystem))
The author of the original code may have written the comment first and then written the code to fulfill the comment however he did not refactor so that the comment could be removed.