Things to look for during a code review

What not to do

Overview

  1. Finding bugs
  2. Create common Coding style
  3. Learning good coding style and best practices

Maintainability

  1. Does the code make sense?
  2. Does the code comply with the accepted Java Conventions?
  3. Does the code comply with the accepted Best Practices?
  4. Does the code comply with the accepted Comment Conventions?

Error Handling

  1. Does the code comply with the accepted Exception Handling Conventions.
  2. Does the code make use of exception handling?
  3. Does the code simply catch exceptions and log them?
  4. Does the code catch general exception (java.lang.Exception)?
  5. Does the code correctly impose conditions for "expected" values?
Person person = Context.getPersonService().getPerson(personId);
person.getAddress().getStreet();
  1. Does the code test all error conditions of a method call?

Security

  1. Does the code appear to pose a security concern?
  2. Service methods should have an @Authorize annotation on them
  3. JSP pages should use the openmrs:require taglib

Thread Safeness

  1. Does the code practice thread safeness?
  2. Does the code avoid deadlocks?

Resource Leaks

  1. Does the code release resources?
  2. Does the code release resources more than once?
  3. Does the code use the most efficient class when dealing with certain resources?

Control Structures

  1. Does the code make use of infinite loops?
  2. Does the loop iterate the correct number of times?

Reusability

  1. Are all available libraries being used effectively?
  2. Are available openmrs util methods known and used?
  3. Is the code as generalized/abstracted as it could be?
  4. Is the code a candidate for reusability?