
Consider only those items that are applicable in your programming environment…
Design
- Code is understandable.
- No magic numbers are used. Instead they are defined in terms of meaningfully named variables that reveal their purpose within the right scope (the lowest scope possible).
- Files/folders/classes/modules/packages are named properly and meaningfully, following conventions of programming languages, frameworks, design patterns, architectural patterns, and architectural styles used.
- Procedures/methods/functions are named properly and meaningfully, following the conventions of programming languages and frameworks used.
- Variables and parameters are named properly and meaningfully, following conventions of programming languages and frameworks used.
- Procedures/methods/functions are small, do only one thing, avoid side effects when possible, and do not return null.
- Number of parameters for each procedure/method/function is minimized, avoiding null and flag arguments (a flag argument tells the function to carry out a different operation depending on its value).
- New objects or variables are minimized in the lowest scopes.
- Objects/variables are duplicated only when necessary.
- Modules encapsulate both data and behavior using ES6 syntax for classes, and provide information hiding by exporting what needs to be visible to the clients.
- Modules are organized in a type hierarchy by using ES6 syntax for inheritance, when appropriate, to support reuse.
- There is no loose code that is not encapsulated in a function.
- Dependency injection is preferred when creating an object’s collaborators, as appropriate.
- OO analysis artifacts are recognizable as distinct modules.
Asynchronous Behavior and Communication
- Callback nesting is kept to a minimal (avoid “callback hell” structures).
- Long, complex anonymous functions are avoided: instead they are defined explicitly (simple and short anonymous functions are fine).
- Promises are used instead of callbacks when appropriate.
- Async-await is used when possible for asynchronous code that uses promises.
- Client-to-server communication is always initiated by an HTTP request, and uses the REST API when not moving between pages (server-to-client communication is initiated by a websocket).
Redundancy
- Code is not replicated in a copy-and-paste style.
- New code does not duplicate existing functionality, but reuses it.
- Functionality is not duplicated in both the front-end code and the back-end end.
- Existing APIs are used when possible.
Error Handling
- Inputs are validated.
- Edge cases are considered (null references, negative values, etc.)
- Object references are verified to be non-null before use.
- Array indexes are checked to avoid out-of-bound errors.
- States of collection data types (list is empty, full) are verified before use.
- Invalid inputs are handled properly and early.
- Possible division by zero is avoided.
- Exception handlers perform the necessary cleanup and recovery functions.
- Resources are released and open connections/sessions are closed when no longer needed.
- Error messages are understandable and complete.
Logic
- Conditions are correct in all control structures (if statements, loops, choice statements).
- Complex conditions are broken down to simpler ones that are easy to understand.
- Loops are guaranteed to terminate.
- Statements that don’t need to be in a loop body are factored out of the loop body. Loops are of minimal length.
- Loop termination conditions are specified correctly.
- Counters and indices are updated at the right place inside control structures.
- Premature returns are not possible.
- There are no unreachable code segments.
- Method chaining (a.b.c.d.e(f)) is minimized (except when using libraries with fluid APIs).
Conventions and Style
- Code is styled according to the idioms and conventions of the programming languages and frameworks used.
- Folder/package structure follows the accepted patterns of the framework and libraries used.
- Code is formatted properly with acceptable spacing and indentation.
Documentation
- Documentation must be updated at the same time as the code (inconsistent documentation is worse than no documentation).
- The purpose of each class/module/package is explained clearly and briefly [unless code is readable and purpose is understandable].
- The purpose of each local/instance and global/class variable is explained clearly and briefly if not self-evident [unless code is readable and purpose is understandable].
- Each procedure/method/function documents the parameters that they need and modify.
- Each procedure/method/function documents their functional dependencies.
- Complex algorithms are explained and justified.
- Code that depends on non-obvious behavior in external libraries is explained.
- Units of measurement for parameters and variables are documented for numeric values.
- There are no needless, obsolete, redundant comments.
- No code is commented out.
- Incomplete, stubbed out code is indicated with appropriate distinctive markers (e.g. “TO-DO” or “FIX-ME”) and examined for its impact (is it forgotten, is it obsolete, could it cause any abnormal behavior, when will it be completed?).
- Comments are consistent in format, length, and level of detail.
References
- Jason Cohen, Steven Teleki, and Eric Brown, “Best Kept Secrets of Peer Code Review”, SmartBear Software.
- http://courses.cs.washington.edu/courses/cse403/12wi/sections/12wi_code_review_checklist.pdf
- http://www.mindfiresolutions.com/Code-Review-Checklist-238.php
- http://www.michalkostic.com/post/104272540521/code-review-checklist
- http://flylib.com/books/en/4.223.1.56/1/
- https://www.liberty.edu/media/1414/%5B6401%5Dcode_review_checklist.pdf
- https://wiki.openmrs.org/display/docs/Code+Review+Checklist
Summary of Carnegie Mellon University – Foundations of Software Engineering, by Professor Cecile Peraire and Professor Hakan Erdogmus