Criteria for high quality code

Code reviews consider the following aspects of code quality:

  • Correctness
  • Program design
  • Efficiency
  • Readability
  • Effective use of comments

Each is described in more detail below.

Correctness

Correctness is primarily established through the extensive use of unit tests. Additional tests may be included in the code review.

Program Design

Well designed code has a structure that is clear, logical, and modular. Here are some specific suggestions.

  • Method bodies are fairly succinct and logical. If a method body has more than 15 lines of code in it, there may be an opportunity to extract out some sub-routines into separate methods.
  • Code is not unnecessarily duplicated: follow the DRY (Don’t Repeat Yourself) principle.
  • Data structures are used appropriately.
  • Encapsulate state. In other words, in most situations, you want to make sure your instance variables are marked as private.
  • Make appropriate use of helper functions to solve natural sub-problems.
  • Arguments to method are appropriate.
  • Replace this: if (/* some expression */) return true else return false; with this: return /* some expression */;

Efficiency

At a minimum, code avoids computation that is strictly unnecessary (e.g., having two for loops that could easily be combined into one). Some assignments may specify additional criteria around efficiency.

Readability

Programs should adhere to good program style. There are many resources online that describe standard practices (e.g., Google’s style guide). While you are not expected to memorize the fine points of any style guide, you should use them as a reference as you code. When in doubt (“do I need curly braces?”), check a reference (answer: yes).

Pay close attention to these particular readability criteria:

  • Print statements: print statements that were added for debugging purposes should be removed.
  • Names (for Variables, Methods, Classes): names for variables should be meaningful and concise. Some very short names are okay if used in the appropriate context. Examples: variables i,j, and k are conventionally used as integer indexes; variables like p,q,r make sense in the context of propositional logic; variables like S, T are appropriate as identifying arbitrary sets. However, avoid using arbitrary short names – such as a, b, c, d – and avoid having similar names refer to objects that have different types (e.g., using var1 to refer to a set and using var2 to refer to an element of the set). Avoid generic names and instead use a name that has meaning in its specific context. For example, suppose you are writing a method that finds that the birth year of the oldest person from a list of persons: avoid using a variable name like returnVal and instead choose something like minBirthYear or ``youngest`. Method and class names should accurately capture their purpose.
  • Indentation and whitespace: Blocks of code should be clearly delineated using consistent indentation. Most editors have the ability to automatically format your code; there are also online tools. Use white space appropriately. Put a blank line between logically separate blocks of code, but avoid having multiple blank lines or inconsistent use of blank lines. Use appropriate horizontal whitespace. Binary operators — e.g., “=”, “+”,— should generally have a space on either side.
  • Curly braces: always use curly braces, even when they are optional. (In Java, curly braces are optional when the code block contains a single line.)

Effective use of comments

Comments should be used for comments only. Do not use comments to comment out code.

Javadocs (if not already provided) can be used to document the purpose of a method.

Comments within the method body should be used sparingly, to highlight the role of certain key steps. Avoid commenting every line and avoid comments that say in words what is obvious from the code. Here is an example of an unnecessary comment.

x = x + 1;   // increment x by one  (<== bad! don't do this!)

Avoid writing comments like that.

A good comment is one that helps the reader see the forest from the trees. A for loop with a complicated body deserves a comment that communicates what this loop accomplishes. Here’s an example:

// find page to evict based on Least Recently Used policy (<== good!)
for (...) {
  ...
  imagine some complex code is here, comparing timestamps on pages, etc.
  ...
}