Checklist for Effective Code Review

0

Are you involved in day-to-day code reviews? Would you like to suggest your team members a checklist that can be used for 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 usecase flows to be considered for review.
  • Maintainability: Maintainability comes as a implied need for every customer when they outsource the 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 following:
      • Unit test coverage: Ask whether there are supportive unit tests for the code. If yes, check for unit test coverage. This is related with the fact that there should be unit tests covering all possible scenarios associated with a usecase. 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.
      • 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 high number of decision points (if/for/while/switch etc), then it may difficult to achieve good test coverage. In that case, the code can be termed as complex, which can further be termed as code which is difficult to change or maintain.
    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 advice for code refactoring.
    3. Duplication: Look out for duplicated code in the class/method and suggest for code refecatoring. The duplicated code makes it difficult to maintain.
  • Usability: Usability of a piece of code is defined in terms of understandability and readability of the code. Usability of a block of code (class/method) can be decided based on following two techniques:
    1. McCabe Code Complexity: Look out for 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 with 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. Following are two different categories:
      • Objects assigned directly via setter/constructor method. One of the most common observations in this relation is storing array directly.
      • Objects returned directly. Objects returned directly may get changed externally which may changed the state of original object that returned it.
    2. SQL injection: There may be possibility of SQL injection where sql query code can be injected into actual query as form of variable. Advice the team to use appropriate manners suggested by specific technology to change the sql query code accordingly.
    3. Validation: Look if appropriate validation of input parameters have taken place. This is more relevant to code associated with controllers/validators.
  • Reliability: From 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.

Ajitesh Kumar

Ajitesh is passionate about various different technologies including programming languages such as Java/JEE, Javascript, PHP, .NET, C/C++, mobile programming languages etc and, computing fundamentals such as application security, cloud computing, API, mobile apps, google glass, big data etc.

Follow him on Twitter and Google+.
Share.

Leave A Reply


8 × = thirty two