Back to Basics – good comments are targeted comments

I can’t think of a single person who enjoys writing comments in code. I don’t, my friends and colleagues don’t, and I’m pretty sure there isn’t a meetup group for fans of it. Outside of code that I write for blog posts, I can pretty much guarantee there are only place where I write comments is in interfaces.

The simple reason for this is that a) interfaces should be explicit, and b) implementations should be self-documenting.

The reason for this blog post is that I’m seeing a lot of code at the moment that falls into one of two traps – either everything is documented, or nothing is documented.

Everything is documented

Spot what’s wrong with this code:

/**
 * The foo class represents blah blah blah...and so on, describing the 
 * class in such detail it's a pity the code couldn't be generated from it
 */
public class Foo {

    /** The name */
    private String name;

   /**
    * Do something with this instance.
    */
   public void doSomething() {
      // Get the name
      String localName = this.getName();

      // munge the local name
      localName.munge();
   }

   /**
    * Get the name.
    * @return the name
    */
   public String getName() {
      // return the name 
      return this.name;
   } 

   /**
    * Set the name.
    * @param name the name
    */
   public void setName(String name) {
      // set the name 
      this.name = name;
   } 
}

Or, to put it another way, spot what’s right with it. That’s a much shorter answer. The code is full of unnecessary comments – e.g. getName() gets the name – and code that seems to have been written just so it could be commented – e.g; String localName – this.getName(); The names have been changed to protect the guilty, but this is real code I’ve seen in a live codebase.

As as I’m concerned, implementations don’t need code-level comments because that’s what the code does anyway.

Nothing is documented

At the other end of the scale is this little gem:

public interface Parser {
    void parse(InputStream is) throws IOException, SQLFeatureNotSupportedException 
}

Interfaces, on the other hand, should have clear documentation that defines what goes in, a generic description of what should happen, and a clear description of what comes out and what exceptions can be thrown. Information at this level should state if, for example, null arguments are allowed, if a null value can be returned, the circumstances in which certain exceptions can be thrown, and so on.

Interfaces should be explicit

Interfaces, to my way of thinking, are contracts, and contracts – as any blood-sucking lawyer can tell you – exist to be honoured. They can’t be honoured if the terms are not explicitly set out.

There are no Burger Kings in Belgium, so if I’m in the UK or the Netherlands I am generally tempted to have one. On my most recent visit, I noticed this at the bottom of the receipt:

“ Free drink with any adult burger with this receipt Excluding Hamburger, Cheeseburger or King deal or any promotional offers”

Or, to put it another way…

/**
 * Get the free drink promised by the receipt.  This is valid for any burger.
 * @param burger the burger
 * @param receipt the receipt
 * @returns the free drink
 * @throws InvalidBurgerException if the burger doesn't qualify for a free drink
 */
public Drink getFreeDrink(Burger burger,
                          Receipt receipt) throws InvalidBurgerException {
    if (MealType.HAMBURGER == meal.type()
        || MealType.CHEESEBURGER == meal.type()
        || MealType.KING_DEAL == meal.type()
        || meal.isPromo()) {
        throw new InvalidBurgerException();
    }
    return new Drink();
}

To my simple brain, this is confusing and contradictory as hell. Your API should be clear and (as my university teachers beat into me) unambiguous – for example, the words “any” and “except” should not appear in the same sentence. Strive for clarity – if you find your API is too hard to clearly document, there’s a good chance it will be annoying to use. In the case above, an improvement would be something along the lines of

/**
 * Get the free drink promised by the receipt.  Not every burger qualifies for a free drink.
 * @param burger the requested burger.  This may or may not qualify for a free drink
 * @param receipt the receipt containing the offer
 * @returns the free drink. May be null, depending on the burger
 */
public Drink getFreeDrink(Burger burger,
                          Receipt receipt) {
    // implementation
}

Note that I’ve also got rid of the exception as a result of being more explicit.

Conclusion

Annoying as it is, documentation is extremely important in the correct place and completely useless everywhere else. Done correctly, they will make your API easier to use and easier to maintain, which is generally a good thing.
 

Reference: Back to Basics – good comments are targeted comments from our JCG partner Steve Chaloner at the Objectify blog.

Related Whitepaper:

Software Architecture

This guide will introduce you to the world of Software Architecture!

This 162 page guide will cover topics within the field of software architecture including: software architecture as a solution balancing the concerns of different stakeholders, quality assurance, methods to describe and evaluate architectures, the influence of architecture on reuse, and the life cycle of a system and its architecture. This guide concludes with a comparison between the professions of software architect and software engineer.

Get it Now!  

3 Responses to "Back to Basics – good comments are targeted comments"

  1. Java Help says:

    Some times the IDE puts that comments not the coders or (wt i think they should) the designers, and maybe can be usefull for that the documentation is not empty (it just seems ugly :S).

  2. Comments are part of the code, so if your IDE inserts them automatically, they should then be

    - removed, if not used
    - completed, if they are useful

    For example, generated getters and setters in IntelliJ have stub comments added. If I’m not going to complete them (and personally, I think comments for getters and setters just add insult to the injury of Java not having real property support), then I delete them.

  3. Mike says:

    First off, those comments on the get and set methods are for the sake of the javadoc. Second, saying that implementation code doesn’t need comments is ridiculous. Sure, this example is so trivial that the comments look stupid, but in a real implementation comments are extremely important for the longevity of your code. I have written every line of my 50k line codebase, and if it wasn’t as well commented as it is, I would have a tough time maintaining the code.

Leave a Reply


five + = 12



Java Code Geeks and all content copyright © 2010-2014, Exelixis Media Ltd | Terms of Use
All trademarks and registered trademarks appearing on Java Code Geeks are the property of their respective owners.
Java is a trademark or registered trademark of Oracle Corporation in the United States and other countries.
Java Code Geeks is not connected to Oracle Corporation and is not sponsored by Oracle Corporation.

Sign up for our Newsletter

15,153 insiders are already enjoying weekly updates and complimentary whitepapers! Join them now to gain exclusive access to the latest news in the Java world, as well as insights about Android, Scala, Groovy and other related technologies.

As an extra bonus, by joining you will get our brand new e-books, published by Java Code Geeks and their JCG partners for your reading pleasure! Enter your info and stay on top of things,

  • Fresh trends
  • Cases and examples
  • Research and insights
  • Two complimentary e-books
Get tutored by the Geeks! JCG Academy is a fact... Join Now
Hello. Add your message here.