Checklist for Effective Code Review

Are you involved in day-to-day code reviews? Would you like to suggest to your team members a checklist that can be used for code reviews? In this blog post, you will learn about key areas to focus on when doing code reviews.

Following is a checklist that one could use while doing code review:

• Functional Suitability: Understand the requirement/use case/user story and ask whether the code you are reviewing meets the requirement or not. This includes the alternate and exception use case flows to be considered for review. Functional suitability is one aspect of code quality that refers to how well the code meets the needs of the user. In order for the software to be functionally suitable, it must be able to perform the tasks that the user requires.
• Maintainability: Maintainability comes as an implied need for every customer when they outsource software development. Following are three key criteria you would want to check for ensuring high maintainability of the code:
  1. Testability: There are two ways of checking the testability of the code. They are the following:
     • Unit test coverage: Ask whether there are supportive unit tests for the code. If yes, check for unit test coverage. This is related to the fact that there should be unit tests covering all possible scenarios associated with a use case. If you find unit tests such as testMethodName, and that it, or testMethod1, testMethod2 without documentation, then there are chances that unit test coverage is not good. That may reflect on complex code which may further reflect on code being difficult to change or maintain. Thus, code can be termed to have low maintainability. You may want to check this blog post on unit test naming convention. You could use Sonar tool to get the information on unit test coverage.
     • McCabe Code complexity: In case you have not found associated unit tests associated with the code because of various reasons, read the code. If the code such as a method has a high number of decision points (if/for/while/switch etc), then it may be difficult to achieve good test coverage. In that case, the code can be termed as complex, which can further be termed as code that is difficult to change or maintain. Recall that McCabe code complexity is a metric for measuring the McCabe Cyclomatic Complexity of a piece of code. It is named after its creator, Thomas McCabe, who developed it in 1976 as a way of measuring the McCabe Cyclomatic Complexity of a program. The McCabe code complexity metric is used by many organizations to determine the code review process and measure code quality. A high McCabe code complexity score indicates that the code is more complex and difficult to understand, while a low McCabe code complexity score indicates that the code is simpler and easier to understand.
  2. Re-usability: Try and understand the functionality being served by method/class. If the method/class is serving multiple functionality or responsibilities, the class may be said to have lower cohesiveness. In that case, the re-usability is low and thus, you may advise for code refactoring.
  3. Duplication: Look out for duplicated code in the class/method and suggest code refactoring. The duplicated code makes it difficult to maintain. Recall that code duplication occurs when the same code is written multiple times, in different parts of the codebase. Code duplication can lead to a number of problems, including increased maintenance costs and decreased code quality.
• Usability: Usability of a piece of code is defined in terms of the understandability and readability of the code. The usability of a block of code (class/method) can be decided based on the following two techniques:
  1. McCabe Code Complexity: Look out for a number of decision points in the class/method. In case the count for decision points in a method go beyond 15 or so, it may be seen as code with high complexity. The code with high complexity may be difficult to read and understand and thus, low usability. Thus, you may want to suggest refactoring for such code.
  2. Documentation: In case, you find documentation to be lower than expected, do raise a concern with the developers to provide documentation for the block of code.
• Security: Following are some common areas for review in relation to security:
  1. Mutability: Try and look for the code which tends to change the state of the class/object. In other words, look for the code which tends to make class mutable. The following are two different categories:
     • Objects are assigned directly via the setter/constructor method. One of the most common observations in this relation is storing the array directly.
     • Objects returned directly. Objects returned directly may get changed externally which may change the state of the original object that returned it.
  2. SQL injection: There may be the possibility of SQL injection where SQL query code can be injected into the actual query as a form of variable. Advice the team to use appropriate manners suggested by specific technology to change the SQL query code accordingly. Recall that SQL injection is a code injection technique, used to attack data-driven applications, in which malicious SQL statements are inserted into an entry field for execution (e.g. to dump the database contents to the attacker). SQL injection must exploit a security vulnerability in an application’s software, for example, when user input is either incorrectly filtered for string literal escape characters embedded in SQL statements or user input is not strongly typed and unexpectedly executed. SQL injection is mostly known as an attack vector for websites but can be used to attack any type of SQL database.
  3. Validation: Look if appropriate validation of input parameters has taken place. This is more relevant to code associated with controllers/validators.
• Reliability: From a software reliability perspective, one would want to check the aspect of failing gracefully. In this regard, check for exception handling vis-a-vis specs and suggest appropriately to include exception scenarios.