Documenting code with Javadoc on waitingforcode.com

Writing a good code includes writing a code well documented. Usually, "well documented" helps programmer to avoid going to see 10 other people before understanding what the code really does.

In this article we'll focus on documenting the code with Javadoc. The article is constructed around an exercise consisting of two parts. At the begin, we'll try to find which elements should contain a good documentation comment. At the second part we'll take the most recent Spring Framework release (4.4.2, at the moment of writing) and try to find fragments matching documentation tips from the first part.

Tips to write a good Javadoc

We could resume the main Javadoc tips under below list (the order is purely subjective):

Be pragmatic - even if some rules are defined, always try to match them into specific context. For example, if one rule tells that comments shouldn't be redundant with the code, but the code structure is composed mainly of conditional clauses, there are nothing wrong to add some quick explaination of if-else results in the comment. We'll constat that in the second part of the article.
Allow code to speak - with well named code, we can easily know what given element is. So, prefer:
```
private static final int LEFT_COORDINATE = 30;
```
Instead of:
```
private static final int L = 30; // left coordinate
```
Explain why and not how - sometimes a single name information is not enough to know the reasons of given behaviour. It's there where comment intervenes. The comment can place the code inside a specific context. For example, a comment can explain to which business rule corresponds this method or why in this method we make some operations. So, consider:
```
/**
 * Calculates the number of solvent customers basing on the 
 * general company rule because the code is not exposed to 
 * anybody else.
 */
```
Rather than:
```
/**
 * Calculates the number of solvent customers with formula:
 * number of customers - number of customers with salary lower than X€
 */
```
Limit redundancy - we should try to avoid comments redundancy. As an example, we can take following snippet:
```
/**
 * Returns reverse name of this {@code User}
 *
 * @return Reverse name of this {@code User}
 */
public String reverseName() {
  // ...
}
```
The comment in the first sentence is the same as after @return tag. In this case, when comment body overlaps with the returning statement, we should only privilege one version.
First sentence role - when we're not very familiar with given library, all available tracks are helpful. One of them is the first sentence from the Javadoc comment. It's important because it represents the content displayed in Javadoc summary tables, as here in the case of Spring Framework Javadoc:

org.springframework.aop Core Spring AOP interfaces, built on AOP Alliance AOP interoperability interfaces.

org.springframework.aop.aspectj AspectJ integration package.

This famous first sentence should be clear, short and meaningful.
Comment if only it brings new information - in the particular case of overridden methods (annotated with @Override) we can tell to see the documentation of parent method. Overridden and implemented method should be documented only if it brings some new behaviour.
Keep comments up-to-date - if comments and code tell opposite things, it's better to not write comments at all. Otherwise, they will necessarily mislead. The reader won't know if it's a comment which is incorrect or the code.
Document public and protected - the general rule is that at least all public and protected methods should be documented. Trivial getter and setters can be excepted of this rule. However, it should be considered as a flexible rule because writing a comment to a method making one simple if-else, with 1-line case content, can be overkill to whole readability.
Nullity - always inform a reader about null values at input and output of given method. By knowing that, method consumer can build a code better protected against NullPointerExceptions. By informing about null possible response, we can also indicate the way of checking if method execution returns valid or invalid object.
Do not be scary to use version related tags - if you're working on public API widely used by programmers community, above all do not under-estimate @version and @since tags. It can be helpful to figure out why given code doesn't work after changing the version. The same rules applied to @deprecated tag which prevents consumer about a possible removal of given code in one of the next releases.
@return is important - not only when we're talking about possible null values, but also in quick method understanding. @return tag should not appear when a method doesn't return anything (void). Its presence in this case is a little bit confusing.
Exceptions - if method signature declares to throw some exceptions, they should be documented with the help of @throws tag.
External information - if some code logic is confusing to other programmers, they can think that it's a bug. To not introduce this communication noise, we can add some external information explaining this logic. It could be, for example, the id of related issue. Even if this number is added in each commit message, it's quicker to take a look directly at code, instead of investigating through all commits to find the right one.
Be a friend with Javadoc language - using appropriated Javadoc tags, such as {@link} when linking to other classes or methods, {@code} in the case of plain-text Java objects, or HTML tags, can lead to a better and quicker project reading. And even for programmers looking directly at source code. With some IDEs, as IntelliJ, they can easily navigate to referenced classes thanks to Ctrl+click shortcut applied on {@link}ed class/method.
Language style - for non-English native programmers, it's also important to known the linguistic rules. From the main important we can distinguish:
- 3rd person in the comments - "gets" instead of "get"
- impersonal form - comments starts by verb in the 3rd person, such as "Provides a new...", "Gets a result of...". This rules applies not only for methods but only for class fields - instead of talking about "This is a form label", we could simply tell that this is "A form label".
- this - the "this" pronoun should be used to refer the fields or methods of current instant. For example, we could document a "set of accepted values of this object" instead of a "set of accepted values of the class".

Comments in Spring Framework - RestGatewaySupport

In the second part of exercise, we'll take two randomly chosen classes, org.springframework.web.client.support.RestGatewaySupport and org.springframework.web.client.MessageBodyClientHttpResponseWrapper, to see theirs good and bad points. Let's begin with shorter one, RestGatewaySupport:

show RestGatewaySupport body

/**
 * Convenient super class for application classes that need REST access.
 *
 * <p>Requires a {@link ClientHttpRequestFactory} or a {@link RestTemplate} instance to be set.
 *
 * @author Arjen Poutsma
 * @since 3.0
 * @see #setRestTemplate
 * @see org.springframework.web.client.RestTemplate
 */
public class RestGatewaySupport {

  /** Logger available to subclasses */
  protected final Log logger = LogFactory.getLog(getClass());

  private RestTemplate restTemplate;


 /**
  * Construct a new instance of the {@link RestGatewaySupport}, with default parameters.
  */ 
  public RestGatewaySupport() {
    this.restTemplate = new RestTemplate();
  }

 /**
  * Construct a new instance of the {@link RestGatewaySupport}, with the given {@link ClientHttpRequestFactory}.
  * @see RestTemplate#RestTemplate(ClientHttpRequestFactory)
  */
  public RestGatewaySupport(ClientHttpRequestFactory requestFactory) {
    Assert.notNull(requestFactory, "'requestFactory' must not be null");
    this.restTemplate = new RestTemplate(requestFactory);
  }


 /**
  * Sets the {@link RestTemplate} for the gateway.
  */
  public void setRestTemplate(RestTemplate restTemplate) {
    Assert.notNull(restTemplate, "'restTemplate' must not be null");
    this.restTemplate = restTemplate;
  }

 /**
  * Returns the {@link RestTemplate} for the gateway.
  */
  public RestTemplate getRestTemplate() {
    return this.restTemplate;
  }

}

As we can see, almost everything is commented. We can see that getter's comment doesn't contain @return tag and the returned object is described directly in comment's body. Globally, the comments are clear. We can see since which version the class can be used and some pre-requisites at the begin of the class definition. One thing little bit confusing for somebody who doesn't know implementation details is a fragment describing default constructor:

Construct a new instance of the {@link RestGatewaySupport}, with default parameters.

We don't necessarily know what are these "default parameters". Are they a part of RestGatewaySupport ? Or maybe they are defined in RestTemplate ? Only a deeper look at RestTemplate class helps to discover that in the reality, the "default parameters" means the definition of message converters:

/**
  * Create a new instance of the {@link RestTemplate} using default settings.
  * Default {@link HttpMessageConverter}s are initialized.
  */
public RestTemplate() {
  this.messageConverters.add(new ByteArrayHttpMessageConverter());
  this.messageConverters.add(new StringHttpMessageConverter());
  this.messageConverters.add(new ResourceHttpMessageConverter());
  this.messageConverters.add(new SourceHttpMessageConverter());
  this.messageConverters.add(new AllEncompassingFormHttpMessageConverter());

  if (romePresent) {
    this.messageConverters.add(new AtomFeedHttpMessageConverter());
    this.messageConverters.add(new RssChannelHttpMessageConverter());
  }
  if (jackson2XmlPresent) {
    messageConverters.add(new MappingJackson2XmlHttpMessageConverter());
  }
  else if (jaxb2Present) {
    this.messageConverters.add(new Jaxb2RootElementHttpMessageConverter());
  }
  if (jackson2Present) {
    this.messageConverters.add(new MappingJackson2HttpMessageConverter());
  }
  else if (gsonPresent) {
    this.messageConverters.add(new GsonHttpMessageConverter());
  }
}

There are another comment which could be improved, the one talking about setter:

Sets the {@link RestTemplate} for the gateway.

It could contain a @throws definition because, after analyzing the method, if restTemplate is null, an IllegalArgumentException is thrown through Assert.notNull(...) precondition. Improved comment could look like:

Sets the {@link RestTemplate} for the gateway. @throws IllegalArgumentException in case of null {@code RestTemplate}

Comments in Spring Framework - MessageBodyClientHttpResponseWrapper

Another inspected class is almost twice bigger than RestGatewaySupport. In consequence, different comment rules were applied there:

show MessageBodyClientHttpResponseWrapper body

/**
 * Implementation of {@link ClientHttpResponse} that can not only check if
 * the response has a message body, but also if its length is 0 (i.e. empty)
 * by actually reading the input stream.
 *
 * @author Brian Clozel
 * @since 4.1.5
 * @see <a href="http://tools.ietf.org/html/rfc7230#section-3.3.3">rfc7230 Section 3.3.3</a>
 */
class MessageBodyClientHttpResponseWrapper implements ClientHttpResponse {

  private final ClientHttpResponse response;

  private PushbackInputStream pushbackInputStream;


  public MessageBodyClientHttpResponseWrapper(ClientHttpResponse response) throws IOException {
    this.response = response;
  } 

  /**
   * Indicates whether the response has a message body.
   * <p>Implementation returns {@code false} for:
   * <ul>
   * <li>a response status of {@code 1XX}, {@code 204} or {@code 304}</li>
   * <li>a {@code Content-Length} header of {@code 0}</li>
   * </ul>
   * @return {@code true} if the response has a message body, {@code false} otherwise
   * @throws IOException in case of I/O errors
   */
  public boolean hasMessageBody() throws IOException {
    HttpStatus responseStatus = this.getStatusCode();
    if (responseStatus.is1xxInformational() || responseStatus == HttpStatus.NO_CONTENT ||
          responseStatus == HttpStatus.NOT_MODIFIED) {
      return false;
    }
    else if (this.getHeaders().getContentLength() == 0) {
      return false;
    }
    return true;
  }

 /**
  * Indicates whether the response has an empty message body.
  * <p>Implementation tries to read the first bytes of the response stream:
  * <ul>
  * <li>if no bytes are available, the message body is empty</li>
  * <li>otherwise it is not empty and the stream is reset to its start for further reading</li>
  * </ul>
  * @return {@code true} if the response has a zero-length message body, {@code false} otherwise
  * @throws IOException in case of I/O errors
  */
  public boolean hasEmptyMessageBody() throws IOException {
    InputStream body = this.response.getBody();
    if (body == null) {
      return true;
    }
    else if (body.markSupported()) {
      body.mark(1);
      if (body.read() == -1) {
        return true;
      }
      else {
        body.reset();
        return false;
      }
    }
    else {
      this.pushbackInputStream = new PushbackInputStream(body);
      int b = this.pushbackInputStream.read();
      if (b == -1) {
        return true;
      }
      else {
        this.pushbackInputStream.unread(b);
        return false;
      }
    }
  }

  @Override
  public HttpHeaders getHeaders() {
    return this.response.getHeaders();
  }

  @Override
  public InputStream getBody() throws IOException {
    return (this.pushbackInputStream != null ? this.pushbackInputStream : this.response.getBody());
  }

  @Override
  public HttpStatus getStatusCode() throws IOException {
    return this.response.getStatusCode();
  }

  @Override
  public int getRawStatusCode() throws IOException {
    return this.response.getRawStatusCode();
  }

  @Override
  public String getStatusText() throws IOException {
    return this.response.getStatusText();
  }

  @Override
  public void close() {
    this.response.close();
  }

}

This time we can see that the comments on getters and setters are omitted. It's because the MessageBodyClientHttpResponseWrapper implements ClientHttpResponse interface where these methods are commented. We can notice here the presence of @see tag which indicates the RFC related to the class content. We can also observe the use of @throws code to indicate possible errors in hasEmptyMessageBody and hasMessageBody methods.

Comment of hasEmptyMessageBody methods is especially precious. If we take a look at code, there are plenty of if-else clauses which are not mandatory the source of quick understanding. But with detailed comment, explaining method result logic, any programmer can quickly answer to any question about this method. It also helps to make a decision which method to use to check if body is empty, between two available: hasMessageBody() and hasEmptyMessageBody().

However, as a negative point of has*Body comments, we can notice the redundancy with code body. These comments don't explain the reasons of implementation choice. Instead, they are more a complement for @return tag and quick explanation of method. They prove that presented rules should be considered in pragmatic way, depending on some context - sometimes comments can explain how the code proceed because, for example, it can help a better understanding.

This article tries to group best practices of writing comments. To explain them in real world, it uses comments of 2 arbitrary chosen Spring Framework classes. Globaly, they both match the majority of explained points. They have very clear first sentence, described error cases and concise description. But keep in mind that comments are only the complements and that the code should be understandable without them.

org.springframework.aop	Core Spring AOP interfaces, built on AOP Alliance AOP interoperability interfaces.
org.springframework.aop.aspectj	AspectJ integration package.