Code Review Checklist - SJC-2015/Tasks GitHub Wiki

Code Review Checklist

Functionality
  • Functionality is implemented in a simple, maintainable, and reusable way (OOP, SOLID, Low Coupling/High Cohesion, KISS, DRY).
Clean Code
  • Use of descriptive and meaningful variable, method and class names as opposed to relying too much on comments. Pick one word per concept. Use Solution/Problem Domain Names.
  • Follow generally accepted code naming conventions and best practices.
  • Class and functions should be small and focus on doing one thing. No code duplications.
  • Methods should not take too many input parameters (up to 3 is fine, 4 is exceptional).
  • Declare the variables with the smallest possible scope. Minimize the scope of local variables.
  • Don’t preserve or create variables that you don’t use again.
  • One return per method body.
  • Omit needless and commented out code - there is a CVS. No System.out.println statements either.
  • Compare objects with equals. Make proper use of equals() and hashCode() methods.
  • Place literals First in comparisons
  • Avoid parameters reassigning.
  • Use thread-safe StringBuffer for string appends. Concatenation is allowed in trivial cases.
Fundamentals
  • Minimize the accessibility of the packages, classes and its members like methods and variables.
  • Code to interface as opposed to implementation.
  • Write fail-fast code by validating the input parameters.
  • Return an empty collection or throw an exception as opposed to returning a null. Be aware of the implicit autoboxing and unboxing cases.
  • Use private fields.
  • Use enums instead of int constants.
  • Use marker interfaces to define types.
Security
  • Sanitize user inputs (for valid data, size, range, boundary conditions, etc).
  • Avoid badly constructed SQL, REGEX, etc. Be aware of SQL Injection.
  • Don’t log sensitive data. Don’t let sensitive information like file paths, server names, host names, etc escape via exceptions.
  • Don't ignore exceptions. No stack trace data should be sent to a client.
  • Don't return nulls. Use exceptions as opposed to return codes.
  • Release resources.
Architecture- and third-party frameworks related
  • Use multi-tiered architecture.
  • Make use of MVC pattern properly.
  • Do not expose DAO to controllers.
  • Anemic Model is fine when it comes to web development.
  • Service layer must be based upon a set of interfaces.
  • Controllers should depend on Service Layer interfaces and Model classes/interfaces.
  • Simplify DAO with Spring and Java Generics.
  • Consider a template engine for view implementation.
  • Inject interfaces, not classes.
  • Avoid scriptlets in JSPs.
  • Make use of common JSP Tag Libraries and Expression Language (EL) for basic needs. Do not reinvent.
  • Keep the View separate from other architecture layers. Use DTOs.
  • It is advised to manage dependencies by using Maven.