wiki:CodeReview

Code reviewing

In this document, we give some guidelines for code reviewing. Please note that these guidelines may not apply to your particular project.

When do I need a code review?

In general, a code review is needed before a program is released. Here are a couple of scenarios:

  • You write a paper that refers to a program you wrote.
  • You release a new version of a program that is in use.

It can also be important to review programs that are used internally:

  • You made a script that is only used to generate results for a paper.

What are the criteria for reviewing code?

It depends on who will use your program. (Adapted from Code Review Checklist). In all cases your program should be checked for:

  • Clarity
    • Is the code clear and easy to understand?
    • Did the programmer unnecessarily obfuscate any part of it?
    • Can the code be refactored to make it clearer?
  • Correctness.
    • Does the program do what it claims to do?
    • If an algorithm is being implemented, is it implemented correctly?
  • Reliability and Robustness
    • Is the code fault-tolerant? Is it error-tolerant?
    • Will it handle abnormal conditions or malformed input?
    • Does it fail gracefully if it encounters an unexpected condition?
  • Maintainability
    • Is the program divided in comprehensible blocks (functions)?
    • Is it well-commented and documented properly?
    • All functions should be documented using a documentation system, e.g., Doxygen, Javadoc, Perldoc, Epydoc, …
    • Inline documentation should be used to explain the problem solved by difficult parts of the program.
  • Security
    • Is the code vulnerable to unauthorized access or malicious use or modification?
  • Scalability
    • Could the code be a bottleneck that prevents the system from growing to accommodate increased load, data, users or input?
  • Efficiency
    • Does the code make efficient use of memory, CPU cycles, bandwidth or other system resources?
    • Can it be optimized?
  • Reusability
    • Could this code be reused in other applications?
    • Can it be made more general?

If your program will also be used by other people, then a decent user interface should be provided. This includes:

  • Argument parsing.
  • A help (usage) function.

There are a number of libraries available to do this automatically for you. One of the most advanced is argparse (for Python). For other languages, similar libraries are available (optparse, getopt, …).

How do I perform code review?

Generalizing, there are two types of code review:

  • Reviewing an existing piece of code in its entirety.
  • Reviewing a set of changes to an existing piece of code.

The former may be most appropriate for (smallish) stand-alone scripts, like a lot of the code we write in our department to support our research experiments.

For larger projects that are developed over a longer time and possibly have more than one person working on them, it is often more useful to do code review on sets of changes as new features are being added.

Note that the two types are not mutually exclusive. For example, you can often learn a lot just by reviewing an existing large body of existing source code. And incidentally, reviewing an existing piece of code could be done by just sending a set of changes containing fixes and inline comments.

In general, you should not spend more than 60-90 minutes at a time on code review (See links below for best practices).

Tool support

Most tools for code review focus on reviewing sets of changes to existing code (i.e. the second type in the previous section). Some examples are:

Our experience shows that the Trac plugin is not very easy to use. If you are interested in rietveld or Review Board, ask me and we may be able to install it on our server.

Also worth mentioning is the git hosting website GitHub (free for Open Source projects) which provides a very intuitive and powerful interface to do code review. See this post for an example of how to use it.

Other resources

For a command line program, having good skeleton code can help a lot. We will collect some samples here.

Some pages of interest:

R/BioConductor programming guidelines:

Last modified 4 years ago Last modified on Nov 29, 2013 3:00:36 PM