Visits: 126

Design

1. Code is understandable.
2. 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).
3. Files/folders/classes/modules/packages are named properly and meaningfully, following conventions of programming languages, frameworks, design patterns, architectural patterns, and architectural styles used.
4. Procedures/methods/functions are named properly and meaningfully, following the conventions of programming languages and frameworks used.
5. Variables and parameters are named properly and meaningfully, following conventions of programming languages and frameworks used.
6. Procedures/methods/functions are small, do only one thing, avoid side effects when possible, and do not return null.
7. 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).
8. New objects or variables are minimized in the lowest scopes.
9. Objects/variables are duplicated only when necessary.
10. 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.
11. Modules are organized in a type hierarchy by using ES6 syntax for inheritance, when appropriate, to support reuse.
12. There is no loose code that is not encapsulated in a function.
13. Dependency injection is preferred when creating an object’s collaborators, as appropriate.
14. OO analysis artifacts are recognizable as distinct modules.

Asynchronous Behavior and Communication

1. Callback nesting is kept to a minimal (avoid “callback hell” structures).
2. Long, complex anonymous functions are avoided: instead they are defined explicitly (simple and short anonymous functions are fine).
3. Promises are used instead of callbacks when appropriate.
4. Async-await is used when possible for asynchronous code that uses promises.
5. 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

1. Code is not replicated in a copy-and-paste style.
2. New code does not duplicate existing functionality, but reuses it.
3. Functionality is not duplicated in both the front-end code and the back-end end.
4. Existing APIs are used when possible.

Error Handling

1. Inputs are validated.
2. Edge cases are considered (null references, negative values, etc.)
3. Object references are verified to be non-null before use.
4. Array indexes are checked to avoid out-of-bound errors.
5. States of collection data types (list is empty, full) are verified before use.
6. Invalid inputs are handled properly and early.
7. Possible division by zero is avoided.
8. Exception handlers perform the necessary cleanup and recovery functions.
9. Resources are released and open connections/sessions are closed when no longer needed.
10. Error messages are understandable and complete.

Logic

1. Conditions are correct in all control structures (if statements, loops, choice statements).
2. Complex conditions are broken down to simpler ones that are easy to understand.
3. Loops are guaranteed to terminate.
4. Statements that don’t need to be in a loop body are factored out of the loop body. Loops are of minimal length.
5. Loop termination conditions are specified correctly.
6. Counters and indices are updated at the right place inside control structures.
7. Premature returns are not possible.
8. There are no unreachable code segments.
9. Method chaining (a.b.c.d.e(f)) is minimized (except when using libraries with fluid APIs).

Conventions and Style

1. Code is styled according to the idioms and conventions of the programming languages and frameworks used.
2. Folder/package structure follows the accepted patterns of the framework and libraries used.
3. Code is formatted properly with acceptable spacing and indentation.

Documentation

1. Documentation must be updated at the same time as the code (inconsistent documentation is worse than no documentation).
2. The purpose of each class/module/package is explained clearly and briefly [unless code is readable and purpose is understandable].
3. 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].
4. Each procedure/method/function documents the parameters that they need and modify.
5. Each procedure/method/function documents their functional dependencies.
6. Complex algorithms are explained and justified.
7. Code that depends on non-obvious behavior in external libraries is explained.
8. Units of measurement for parameters and variables are documented for numeric values.
9. There are no needless, obsolete, redundant comments.
10. No code is commented out.
11. 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?).
12. 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