Clean Code by Robert C. Martin Quick Tips

I’ve recently been reading the book Clean Code by Robert C. Martin aka Uncle Bob which fantastically captures many programming paradigms, useful information and tips to improving your development skills. Whilst we were at our offsite, I led a talk as an overview for things we can start introducing or doing as a team. Below are my notes I made for this talk, which you can read in 15-20 minutes to pick up some useful information!

Cost of a Mess

  • Changes become hard, each change will often result in breaking something else,
  • Reading code often increases the length to understand or decipher the intention or meaning,
  • Mess becomes so big, it becomes impossible to refactor,
  • Increasing amount of mess reduces team productivity - a developer does not want to spend 90% of their time cleaning code. We’d much rather do fun new things!
  • Overhaul of a product takes an incredible amount of time and resources. Often this is what can cause a business to fail, the new product will be required to meet the same specification as the legacy product. This results in:
  • Development process of the new product taking years,
  • Behind competitors in terms of features,
  • Original development team may have left a few years into the redesign, which will then further pivot the original intentions and result will end up in just as much of a mess.

Naming

Intention revealing names

  • Good naming take time, but they arguably save more time,
  • It should tell you why it exists, what it does and how it is used,
  • Comments invalidate this rule! If you use a comment to explain this - then you not clearly explained your intent,
int d //time elapsed in days

int elapsedTimeInDays

Let’s look at a further example, can you guess what this is for?

Public List<int[]> getThem() {
  List<int[]> list1 = new ArrayList<int[]>();
  
  for (int[] x : theList) {
    if (x[0] == 4) {
      list1.add(x)
      return list1
    }
  }
}

What kind of things are in theList? What is the significance of the zeroth subscript of an item in theList? What is the significance of the value 4? How would I use the list being returned?

Answer: minesweeper

Now compare this to:

Public List<Cell> getFlaggedCells() {
  List<Cell> flaggedCells = new ArrayList<Cell>();
  
  for (Cell cell : gameBoard) {
    if (cell.isFlagged()) {
      flaggedCells.add(cell)
      return flaggedCells;
    }
  }
}

Simply by caring about the names, it makes it much easier to understand the intention

Avoid Disinformation

  • Abbreviations may look good at the time, but not everyone understands them - you have context but someone who is looking at this for the first time may not Example: var acc, this could mean ‘accumulator’, ‘accumulate’, ‘account’ - you need to read back to find out context,
  • Do not refer to something that is not correctly return type - accountList is often used to describe a grouping of accounts but is not actually of type List

Make Meaningful Distinctions

  • Variables should never be renamed just to be sufficient enough for the compiler just because you can’t reference by the same name in the same scope, If names must be different, then they should also mean something different,
  • Number series naming (a1, a2, … aN) is the opposite of intentional naming. They are non-informative.
public static void copychars (char a1[], char a2[]) {
  for (int i = 0; i < a1.length; i++) {
    a2[i] = a1[i];
  }
}

It is not indicative of which is being copied, and to where. If you were to call this function which argument should you put the source and the destination? You can solve this issue by renaming it to the latter.

Use Pronounceable and Searchable Names

  • Made up words are not friendly to new developers, e.g. genymdhms (generation date, year, month, day, hour, minute and second)
  • Using MAX_CLASSES_PER_STUDENT is easier to find than the number 7,
  • Letters are a poor choice for any variable, using the letter e makes it impossible to search (event is much easier)

Hungarian Notation

This was used in name-length-challenged languages, early versions of BASIC only allowed one letter plus one digit. Longer examples of this still exist:

  • PhoneNumber phoneString; string is not necessary - you already have a type; this obfuscates code and can conclude to questioning unnecessary code

Class Names

  • Classes and objects should have a noun or noun phrase names, like customer, account and addressparser. Avoid words like manager, processor, data or info in the name. Class names should not be a verb.

Method Names

  • Methods should have a verb or verb phrase names like postPayment, deletePage, or save. Accessors, mutators and predicates should be named for their value and prefixed with get, set and is.

Functions

‘You know you are working on clean code when each routine turns out to be pretty much what you expected.’

  • Functions should ‘do one thing, they should do it well and do it only’,
  • If you can extract another function from a function with a name that is not merely a restatement of its implementation then it is not doing one thing,
  • Indent structure should not be greater than one or two,
  • Switch statements should be avoided, but it is not always possible (use polymorphism) refer to page 38/39,
  • Use descriptive names - it is hard to overestimate the value of good names,
  • Don’t be afraid to make a name long. A long descriptive name is better than a short enigmatic name. It is better than a long descriptive comment.
  • Don’t be afraid to spend time choosing a name. Always improve when you think of a better name, it is easy to change names via editors.
  • Be consistent in naming choices,
  • Code should read like a top-down narrative

Example:

getUser: function(key)

What do you think this does?

  • Get user by an id?
  • Maybe they forgot to call it get userid?

This function actually: Gets a key’s value from local storage for the user who has an active session

Arguments

  • Ideally 0 arguments in a function, but no more than three; should be avoided where possible,
  • Arguments make test cases complicated; the more arguments, the more testing involved.

Monadic functions (one argument)

  1. Asking a question about the argument; returning a value, e.g. fileExists(“myFile”)
  2. Operating on the argument and returning a value, e.g. openFile(“myFile”)
  3. An event, where there is an input arg but no output

Dyadic Functions (two arguments)

Take the example of writeField(name) and writeField(output, name). Although the meaning of both is clear, when you read the second you glance past the first argument. Then stop for a second. The problem here is that you are ignoring code, and you should never ignore code! Parts we ignore, is where bugs will hide.

  • Use two arguments where natural cohesion or natural ordering occur, e.g. new Point(0,0); this is ok because points are ordered components of a single value.

Triads (three arguments)

  • Introduces issues of ordering, pausing and ignoring code,
  • You can reduce the number of arguments by creating objects out of them,
  • Argument lists (using destructuring i.e. …), but are considered one argument when they are equivalent

Have No Side Effects

  • If your function is doing something more than what it is named, then you have introduce side effect into your code. It creates temporal coupling. Example:
  • isPastEvent() is a function used in rules engine, which returns value. But it also sets the position number if it is not defined. This is a side effect, as you would expect a return value but not to adjust state.

Output Arguments

  • appendFooter(s), does this append to s or does it append s to something?
  • Output arguments should be avoided, if your function must change state of something, have it change the state of its owning object.

Flag Arguments

  • Passing flag arguments indicates the function does more than one thing. E.g. Render(true) is extremely confusing, you would always expect it to render when calling render!

Command Query Separation

  • Functions should either do something or answer something, but not both.

Prefer Exceptions to Returning Error Codes

  • Refer to page 46
  • Extract catch block, so the developer does not need to concern themselves with handling error object

Comments

When to avoid comments

  • Common motivations for writing comments is bad code. Clear and expressive code with few comments is far superior to cluttered and complex code with lots of comments.
  • Creating smaller functions for code can result in better explanation (see pg 55),
  • Misleading comments - if it is not precise enough to be accurate it can waste more time for the next person who reads it,
  • Mandated comments just for the sake of it - clutters code and propagates lies,
  • Noise comments that restate the obvious,
  • Don’t use comments when you can use a function or variable (see pg 67),
  • Position markers are equivalent to noise (e.g. // actions //////////////),
  • Commented out code - others who see it won’t have the courage to delete it. They’ll think it’s there for a reason, or it’s too important to delete. Source control saves our butt, so just delete it!

When are comments ok to use?

  • Copyright and authorship statements (ales does this)
  • To explain formatting of dates as they are generally unclear,
  • Explanation of intent - why a decision was made, although we may not agree with implementation we can see why a particular part was necessary,
  • Translate obscure argument or return value into something that’s readable, although it is better to make the value clear if possible,
  • Warning of consequences - changing a value may result in undesired behaviour, for example why a particular test is disabled,
  • TODO comments - explaining the future of the function or class,
  • Amplification, explain the importance of something that may be inconsequential

Takehome Principles

  1. Boy scout - ‘leave the campground cleaner than you found it’,
  2. Step down - code is a story, it should read like a narrative,
  3. Reduce - remove as many arguments from a function where possible