OSEE/Software Development Process/Peer Review Checklist
- Has the standard formatter been applied?
- Are methods and classes within size guidelines?
- Are coding guidelines checked with tools?
- Are descriptive variable and constant names used in accord with naming conventions (camel)?
- Are all variables properly defined with meaningful, consistent, and clear names?
- Is every variable correctly typed?
- Is every variable properly initialized?
- Should any of the variables be constants (final)?
- Should any of the fields be local variables?
- Do all fields have appropriate access modifiers (private, protected, public)?
- Should any static fields be non-static, or any non-static fields be static?
- Do any variables have confusingly similar names as other variables?
- Are variables defined close to where they are used?
- Are variables used defensively (add link)?
- Are divisors tested for zero?
- Do comparisons of floating point numbers have tolerance?
- Are descriptive method names used in accord with naming conventions?
- Do all methods have appropriate access modifiers (private, protected, public)?
- Should any static methods be non-static, or any non-static methods be static?
- Do methods have a clear flow and return criteria?
- Do method names describe the entire functionality?
- Does the method provide one function?
- Is an empty set returned instead of null (Collections.empty…)?
- Does each class have an appropriate constructor?
- Does the constructor only perform initialization of parameters?
- Are constructors free of exceptions ?
- Do any subclasses have common members that should be in the superclass?
- Can the class inheritance hierarchy be simplified?
- Are any modules excessively complex and need to be restructured or decomposed?
- Do class names reflect the functionality they provide?
- Is the base class only using methods of or dependent on a parent class?
- Does the class have a clear singular responsibility?
- Does the class depend on an abstraction not a concretion?
- Does the class have a clear lifecycle?
- Can better data structures or more efficient algorithms be used?
- Are logical tests arranged such that the often successful and inexpensive tests precede the more pensive and less frequently successful tests?
- Can the cost of re-computing a value be reduced by computing it once and storing the results?
- Is every result that is computed and stored actually used?
- Can a computation be moved outside a loop?
- Do all tests within a loop need to be performed?
- Can multiple loops, operating on the same data, be combined into one loop?
- Are intermediate objects, used in computations, scoped appropriately so they can be garbage collected quickly when no longer needed?
- Are all resources closed after use (files, images… etc.)[using try…finally]?
- Is the read buffer of a resource appropriately sized?
- Does the reading of resources take into account memory usage [streaming]?
- Is I/O access minimized using an appropriate streaming mechanism?
- Are any variables or attributes redundant or unused?
- Does the code contain any uncalled or unneeded methods or leftover code stubs?
- Can any code be replaced by calls to external reusable objects?
- Are any blocks of code repeated that could be condensed into a single method?
- Do modules have a low level of coupling between them (methods and classes)?
- Do modules have a high level of cohesion within each one (methods or class)?
- Is dependency injection used instead of singletons? (singletons hide dependencies)
- Do the comments help understand the code?
- Are all comments consistent with the code?
- Do the commit comments follow the convention?
- Does the code contain sufficient, descriptive, and well written comments?
- Are comments only used for technical notes?
- Are any code segments commented out?
- Are all for-loop control variables declared in the loop header?
- Is the best choice of looping constructs used?
- Will all loops terminate?
- When multiple exits from a loop are used, is each exit necessary and handled properly?
- Does each switch statement have a default case?
- Are missing switch case break statements correct and marked with a comment?
- Is the nesting of loops and branches too deep, and is it correct?
- Are conditionals checking for the positive?
- When ‘goto’ statements (continue, break, or multiple returns) are used, is each necessary and handled properly?
- Is polymorphism used to operate on items using a single if/else/switch to construct polymorphic objects instead of using if/else/switch each time we want to manipulate items of that type?
- Are caught exceptions handled in a meaningful way?
- When exceptions are swallowed, is the exceptional case completely resolved? Was the system left in a consistent state?
- Are Exceptions only used for signaling error conditions and not flow control, which causes bad performance?
- Are runtime and checked exceptions and throwable java.lang.error considered and handled appropriately?
- Is the constant or configurable item acquired by a centralized object?
- Does the item need to be configurable?