A Code review is a way of validating the design and implementation of an application before it goes live. This is not the QA process or but a high level approach of allowing the developer to ensure there is a level of consistency in the coding of the application. This can be accomplished by utilizing a checklist which details various checks to make before the final product goes live.
The reviewer should be someone with domain expertise in the problem area. A reviewer may also utilize other areas of his or her expertise and comment on other possible improvements. There is no gurantee that the person reviewing the code will be familiar with the application therefore it is always good to have the original developer, if possible, present to perform a walk through of the application to give the reviewer a better idea of the application and its internal workings.
————
Checklist for Code Reviews
Requirements Traceability
- Does the code implement the design/bug fix?
- Does the developer understand requirements being implemented?
- Does the code execute as expected?-
- Has the developer tested overall functionality?
- Has the error-handling code been tested?
- Is there any code included that is not directly associated to the requirements?
Resource Leaks
- Is every memory allocation de-allocated?
- Is the code written in a way that it has impact on memory usage?
- Does the code close database connections
- Does the code close HTTP connections
- Does the code release objects from memory
- Are objects freed up after an error?
- Are sessions properly closed and not hanging?
Maintainability
A. Structure
- Is the code well-structured, consistent in style, and consistently formatted?
- Are there any extraneous procedures or unreachable code?
- Are there any leftover stubs or test routines in the code?
- Can any code be replaced by calls to external reusable components or library functions?
- Are there any blocks of repeated code that could be condensed into a single procedure?
- Are any modules excessively complex and should be restructured or split into multiple
routines?
B. Documentation
- Is the code clearly and adequately documented with an easy-to-maintain commenting style?
- Are all comments consistent with the code?
- Are variable declarations properly commented?
- Are functions, methods, components, classes described and documented?
- Are comments used to identify missing functionality or unresolved issues?
Variables
- Are all variables properly defined with meaningful, consistent, and clear names?
- Do all assigned variables have proper type consistency or casting?
- Are there any redundant or unused variables?
- Are variables scoped correctly?
- Are variables initialized before they are used?
- Are global variables thread-safe?
Control Structures
- Are all loops, branches, and logic constructs complete, correct, and properly nested?
- Are the most common cases tested first in IF- -ELSEIF chains?
- Are all cases covered in an IF- -ELSEIF or CASE block, including ELSE or DEFAULT clauses?
- Does every case statement have a default?
- Are loop termination conditions obvious and invariably achievable?
- Are indexes or subscripts properly initialized, just prior to the loop?
- Can any statements that are enclosed within loops be placed outside the loops?
- Does the code in the loop avoid manipulating the index variable or using it upon exit from the loop?
Defensive Programming
- Are indexes, pointers, and subscripts tested against array, record, or file bounds?
- Are imported data and input arguments tested for validity and completeness?
- Are all output variables assigned correctly?
- Are timeouts or error traps used for external interfaces?
- Are files checked for existence before attempting to access them?
- Are all files left in the correct state upon program termination?
- Does the code avoid Deadlocks?
Error Handling
- Are errors properly handled each time a function returns?
- Where are the resulting errors being handled
- Has the error-handling code ever been tested
- Does the error checking follow error handling standards
Arithmetic Operations
- Are divisors tested for zero?
- Does the code systematically prevent rounding errors?
- Does the code avoid additions and subtractions on numbers with greatly different magnitudes?
Security
- Validate input from all untrusted data sources?
- Is data Sanitized through url/form posts
- Sanitize data sent to other systems?
- Does the code hard code server or user sensitive information
- Does the application time out
- Are sessions killed after logout
Performance/Execution
- Does the application meet performance requirements?
- Is there any code, which could create unintended infinite loops?
