Comments
Always try to explain yourself in code.
If our programming languages were expressive enough, or if we had the talent to subtly wield those languages to express our intent, we would not need comments very much—perhaps not at all.
Inaccurate comments are far worse than no comments at all. They delude and mislead. They set expectations that will never be fulfilled. They lay down old rules that need not, or should not, be followed any longer.
Truth can only be found in one place: the code. Only the code can truly tell you what it does. It is the only source of truly accurate information. Therefore, though comments are sometimes necessary, we will expend significant energy to minimize them.
Rather than spend your time writing the comments that explain the mess you’ve made, spend it cleaning that mess.
// Check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) && (employee.age > 65))
if (employee.isEligibleForFullBenefits())
Sometimes our corporate coding standards force us to write certain comments for legal reasons, for example, copyright and authorship.
// Copyright (C) 2003,2004,2005 by Object Mentor, Inc. All rights reserved.
// Released under the terms of the GNU General Public License version 2 or later.
In this case the comment lets us know that the regular expression is intended to match a time and date.
// format matched kk:mm:ss EEE, MMM dd, yyyy
Pattern timeMatcher = Pattern.compile(“\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*”);
Sometimes a comment goes beyond just useful information about the implementation and provides the intent behind a decision.
// This is our best attempt to get a race condition
// by creating a 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();
}
Sometimes it is just helpful to translate the meaning of some obscure argument or return value into something that’s readable. In general, it is better to find a way to make that argument or return value clear in its own right; but when its part of the standard library, or in code that you cannot alter, then a helpful clarifying comment can be useful.
assertTrue(a.compareTo(a) == 0); // a == a
assertTrue(a.compareTo(b) != 0); // a != b
assertTrue(ab.compareTo(ab) == 0); // ab == ab
Sometimes it is useful to warn other programmers about certain consequences.
// Don't run unless you have some time to kill.
public void _testWithReallyBigFile() {
writeLinesToFile(10000000);
String responseString = output.toString();
assertSubString("Content-Length: 1000000000", responseString);
assertTrue(bytesSent > 1000000000);
}
public static SimpleDateFormat makeStandardHttpDateFormat() {
// SimpleDateFormat is not thread safe,
// so we need to create each instance independently.
SimpleDateFormat df = new SimpleDateFormat(”EEE, dd MMM yyyy HH:mm:ss z”);
df.setTimeZone(TimeZone.getTimeZone(”GMT”));
return df;
}
TODOs are jobs that the programmer thinks should be done, but for some reason can’t do at the moment. It might be a reminder to delete a deprecated feature or a plea for someone else to look at a problem. It might be a request for someone else to think of a better name or a reminder to make a change that is dependent on a planned event. Whatever else a TODO might be, it is not an excuse to leave bad code in the system.The
TODO comment explains why the function has a degenerate implementation and what that function’s future should be.// TODO-MdM these are not needed
// We expect this to go away when we do the checkout model
protected VersionInfo makeVersion() throws Exception {
return null;
}
There is nothing quite so helpful and satisfying as a well-described public API. The java-docs for the standard Java library are a case in point. It would be difficult, at best, to write Java programs without them.
/**
* A subclass of {@link MimeType} that adds support for quality parameters
* as defined in the HTTP specification.
*
* @author Arjen Poutsma
* @author Juergen Hoeller
* @since 3.0
* @see <a href="https://tools.ietf.org/html/rfc7231#section-3.1.1.1">
* HTTP 1.1: Semantics and Content, section 3.1.1.1</a>
*/
public class MediaType extends MimeType implements Serializable {
private static final long serialVersionUID = 2069937152339670231L;
/**
* Public constant media type that includes all media ranges (i.e. "*/*").
*/
public static final MediaType ALL;
}
A comment is redundant if it describes something that adequately describes itself. Comments should say things that the code cannot say for itself.
// Create a directory
public void createDirectory(String name) {
}
public abstract class ContainerBase {
/**
* The background processor delay for this component.
*/
protected int backgroundProcessorDelay = -1;
/**
* The lifecycle event support for this component.
*/
protected LifecycleSupport lifecycle = new LifecycleSupport(this);
}
// 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");
}
}
}
/** The version. */
private String info;
The method does not return when
this.closed becomes true. It returns if this.closed is true; otherwise, it waits for a blind time-out and then throws an exception if this.closed is still not true.It is just plain silly to have a rule that says that every function and variable must have a comment. Comments like this just clutter up the code.
/**
* Add CD
*
* @param title The title of the CD
* @param author The author of the CD
* @param numberOfTracks The number of tracks on the CD
* @param durationInMinutes The duration of the CD in minutes
*/
public void addCD(String title, String author, int numberOfTracks, int durationInMinutes) {
}
Sometimes people add a comment to the start of a module every time they edit it. These comments accumulate as a kind of journal, or log, of every change that has ever been made. Nowadays, however, we have source code control systems that did it for us. They should be completely removed.
* 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);
* 05-Dec-2001 : Fixed bug in SpreadsheetDate class (DG);
* 29-May-2002 : Moved the month constants into a separate interface
* (MonthConstants) (DG);
* 27-Aug-2002 : Fixed bug in addMonths() method, thanks to N???levka Petr (DG);
* 03-Oct-2002 : Fixed errors reported by Checkstyle (DG);
* 13-Mar-2003 : Implemented Serializable (DG);
* 29-May-2003 : Fixed bug in addMonths method (DG);
* 04-Sep-2003 : Implemented Comparable. Updated the isInRange javadocs (DG);
* 05-Jan-2005 : Fixed bug in addYears() method (1096282) (DG);
Noise comments restate the obvious and provide no new information.
/**
* Default constructor.
*/
protected AnnualDateRule() {
}
/** The day of the month. */
private int dayOfMonth;
/**
* Returns the day of the month.
*
* @return the day of the month.
*/
public int getDayOfMonth() {
return dayOfMonth;
}
// does the module from the global list <mod> depend on the
// subsystem we are part of?
if (smodule.getDependSubsystems().contains(subSysMod.getSubSystem()))
List<Module> moduleDependees = smodule.getDependSubsystems();
String ourSubSystem = subSysMod.getSubSystem();
if (moduleDependees.contains(ourSubSystem))
There are rare times when they make sense, but in general they are clutter that should be eliminated.
/////////////////////////////////////////////////
// PROPERTIES, PRIVATE AND PROTECTED
/////////////////////////////////////////////////
private String name;
private int age;
protected void doSomething() {
showInfo();
}
private String showInfo() {
}
/////////////////////////////////////////////////
// PUBLIC
/////////////////////////////////////////////////
public void save() {
}
If you find yourself wanting to mark your closing braces, try to shorten your functions instead.
public class App {
public static void main(String[] args) {
try {
while ((String line = in.readLine()) != null) {
} // End while
} // End try
catch (IOException e) {
} // End catch
finally {
} // End finally
} // End main
} // End class
It makes the comments hard to read in the one place where they should be easy to read—the editor/IDE.
/**
* Task to run fit tests.
*
* <p>
* This task runs fitnesse tests and publishes the results.
* <p/>
*
* <pre>
*
* Usage:
*
* <taskdef name="execute-fitnesse-tests"
*
* classname="fitnesse.ant.ExecuteFitnesseTestsTask"
*
* classpathref="classpath" />
*
* </pre>
*/
Few practices are as odious as commenting-out code. Don’t do this!
InputStreamResponse response = new InputStreamResponse();
response.setBody(formatter.getResultStream(), formatter.getByteCount());
// InputStream resultsStream = formatter.getResultStream();
// StreamReader reader = new StreamReader(resultsStream);
// response.setContent(reader.read(formatter.getByteCount()));
Don’t put interesting historical discussions or irrelevant descriptions of details into your comments.
/*
RFC 2045 - Multipurpose Internet Mail Extensions (MIME)
Part One: Format of Internet Message Bodies
section 6.8. Base64 Content-Transfer-Encoding
The encoding process represents 24-bit groups of input bits as output
strings of 4 encoded characters. Proceeding from left to right, a
24-bit input group is formed by concatenating 3 8-bit input groups.
These 24 bits are then treated as 4 concatenated 6-bit groups, each
of which is translated into a single digit in the base64 alphabet.
When encoding a bit stream via the base64 encoding, the bit stream
must be presumed to be ordered with the most-significant-bit first.
That is, the first bit in the stream will be the high-order bit in
the first 8-bit byte, and the eighth bit will be the low-order bit in
the first 8-bit byte, and so on.
*/
Short functions don’t need much description. A well-chosen name for a small function that does one thing is usually better than a comment header.
/**
* Save product only, if the given object is not a product, then throw a ClassCastException
*
* @param obj The product
*/
public void save(Object obj) {
Product product = (Product)obj;
ProductEntity productEntity = mapToEntity(product);
productRepository.save(productEntity);
}
public void saveProduct(Product product) {
ProductEntity productEntity = mapToEntity(product);
productRepository.save(productEntity);
}
As useful as Javadocs are for public APIs. Generating Javadoc pages for the classes and functions inside a system is not generally useful, and the extra formality of the Javadoc comments amounts to little more than cruft and distraction.
If you must write a comment, then make sure it describes the code it appears near. Don’t offer systemwide information in the context of a local comment.
The following example, aside from the fact that it is horribly redundant, also offers information about the default port.
/**
* Port on which fitnesse would run. Defaults to 8082.
*
* @param fitnessePort
*/
public void setFitnessePort(int fitnessePort) {
this.fitnessePort = fitnessePort;
}