Graduate Program KB

Clean Code

Chapter 4

  • Comments are used to compensate our failure to express ourself in code

    • Failure to select good descriptive names
    • Failure to structure our code properly

  • The purpose of comments is to clarify the code's intent, not reiterate what it obviously does

  • Comments often don't paint the full picture, they can lie

    • As code continues to develop, the description of the comment also has to be updated along the way
    • The behaviour you're clarifying for some code must be very specific, making comments often large
    • The comments will become degenerate, old, difficult to maintain and inaccurate

Types of comments

  • Explanatory comments

    • Often not necessary if you're employing good naming techniques for your variables
    • It's much easier to glance at some code and realise its intent
    // Check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) &&
    (employee.age > 65))

    if (employee.isEligibleForFullBenefits())

  • Legal comments

    • Sometimes corporate coding standards forces statements for copyright and authorship to be written at the top of a file
    • Instead, they should be referred to a standard license or external document rather than putting the terms and conditions into the comment

  • Clarification comments

    • Sometimes it's helpful to translate the meaning of an obscure argument or return value to something readable
    • Ensure that there is no other alternative before commenting, since it can be difficult to verify that a clarifying comment is correct
    assertTrue(a.compareTo(a) == 0);    // a == a
assertTrue(a.compareTo(b) != 0);    // a != b
assertTrue(ab.compareTo(ab) == 0);  // ab == ab
assertTrue(a.compareTo(b) == -1);   // a < b
assertTrue(aa.compareTo(ab) == -1); // aa < ab
assertTrue(ba.compareTo(bb) == -1); // ba < bb
assertTrue(b.compareTo(a) == 1);    // b > a
assertTrue(ab.compareTo(aa) == 1);  // ab > aa
assertTrue(bb.compareTo(ba) == 1);  // bb > ba

  • TODO comments

    • Used to indicate areas of code which need development or improvement
    • Having a TODO comment indicates bad code is being left in the system, which adds complexity to the reader
    • IDEs provide features to locate TODO comments quickly but code should not be littered with TODOs

  • Amplification comments

    • Used to emphasis the importance of something that may seem insignificant at a first glance

  • Redundant comments

    • Don't put comments for the sake of putting comments
    • Variable or function names should be descriptive and simple enough that you should be able to understand its purpose by reading it
    // This function adds two values
public int add(int firstNumber, int secondNumber) {
    return firstNumber + secondNumber;
}

  • Misleading comments

    • As previously mentioned, comments must be specific. Otherwise, they can become misleading
    • If the behaviour of some code isn't described precisely enough, then it can lead to the reader becoming surprised if an edge case produces an unexpected result

  • Mandated comments

    • Large comment blocks above functions which dictate its imports, exports and behaviour are unnecessary
    • Imagine a file with lots of functions and their associated comment blocks, the file will become cluttered and difficult to read

  • Journal comments

    • People used to log updated changes to the code by appending comments over time
    • Nowadays, with version control systems like git in place, we don't need these large accumulating comments to be maintained in our code
    * 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);

  • Commented-out code

    • As code evolves throughout development, some functionality might become redundant which leads to developers commenting code out
    • Often times, they don't want to delete it incase it could be important in the future, however, leaving these implementations can lead to confusion
    • With version control systems widely used, we can save a snapshot of our code and revert back if anything terrible ever happens, so you can safely delete the code
    int numberOfApplesToBuy;
// boolean areThereApples = checkIfThereAreApples();
addNumberOfApplesToCart(numberOfApplesToBuy);

  • HTML comments

    • HTML in source code comments makes it hard to read in the IDE, which is counterintuitive because that's our development environment
    • The HTML is rendered to be viewed nicely on the browser, however that functionality should be the responsibility of the tool and not the programmer

General guideline for good commenting

  • If you are going to write comments, make it the best comment it can be. Use concise language and keep it short as possible
  • Comments should not regurgitate the functionality of a piece of code that already clearly expresses its intent
  • Don't clutter your comments with historical discussions or irrelevant descriptions, simply state what the code does
  • Regularly review and maintain comments as the behaviour of your program changes, you will often find code can be refactored into different functions 
    // does the module from the global list <mod> depend on the
// subsystem we are part of?
if (smodule.getDependSubsystems().contains(subSysMod.getSubSystem()))

    ArrayList moduleDependees = smodule.getDependSubsystems();
String ourSubSystem = subSysMod.getSubSystem();
if (moduleDependees.contains(ourSubSystem))