Code Review Checklist
(Refer to Ebenezar John Paul's writing on the Internet)
An Ideal Code Review Checklist that applies for most programming languages
Introduction
The purpose of this article is to propose an ideal and simple checklist that can be used for code review for most languages. Even though there are a lot of code review techniques available everywhere along with how to write good code and how to handle bias while reviewing, etc., they always miss the vital points while looking for the extras. The main idea of this article is to give straightforward and crystal clear review points for code reviewers to review efficiently with least time possible.
Purpose of Code Review
The ultimate purpose of code review is to investigate the code to find weak spots, faults, strengths and ways to optimize the code for better performance. It is mainly to deliver a bug-free (at least near perfect) application that meets the purpose (requirements) while meeting the industry standards.
The code review checklists are illustrated in two parts:
Code Review Checklist - Fundamental
Code Review Checklist - Comprehensive
Code Review Checklist - Fundamental
1) Objective based [Purposeful]
The code achieves its purpose. In simple terms, it does what it is supposed to.
2) Unbreakable [Validated]
Validations are used wherever necessary. The code never breaks under any circumstances. Especially under invalid inputs that come from the user end. Regardless of it being a negative, over-sized, invalid format, etc., every input passed should be processed, sanitized before taking it further. Every object is checked for its actual data existence before accessing its properties.
3) Responses are handled [Error handling and Data formatting]
Not just the error messages, every response that is returned by the server must be properly handled. It should have necessary headers, response messages, error codes and any other necessary details attached with it in required format. All possible scenarios are tested to avoid deadlocks, timeouts, etc.
4) Follows architecture [No design deviation]
Verify that the approved architecture/design is followed throughout the application (If there is none, consider putting it in place). If there are any design changes required, ensure that these are documented, baselined and approved before implementing them in the existing code.
5) Unit tested [Reliable]
Every core method has a unit test which passes.
6) Reusable [No repetition]
All methods serve a limited and clear purpose (follows DRY principle). Functions are reused wherever applicable and written in such a way that they can be re-used in the future implementations. There is no duplication of code. Logics make use of general functions without ambiguity.
7) Performance oriented [Speedy response & Scalability]
The landing of the application is swift. There are no long delays between the requests and responses. Raw string concatenations are avoided and proper methods such as StringBuilder are used. The code is scalable and able to handle a large amount of data and upcoming features.
8) Secure [Safe code]
The code is secure in terms of authentications (with encryption), injections, roles, unauthorized access, directory browsing, SQL injection, cross-side scripting, etc. It follows the OWASP 10 security principles.
Code Review Checklist - Comprehensive
9) Manageable [Crisp and Formatted]
The code is readable, commented and easy to manage. It is friendly formatted and easy to read/understand. Methods are not too big to manage and they don't exceed readable size.
10) Meets coding conventions and standards [Standardized approach]
The code follows the coding conventions, standards and is consistent with the existing application code. There are no commented code and hard coded values.
11) Detach connections after usage [Memory handling]
Resources that are not automatically released after usage are freed. Connections, ports are closed properly.
12) Has configurable logging[Traceability]
Log every transaction or the ones that require logging. They are stored in a repository (as a file) as well as in the database (as text). Logging in different stages for different purposes can be enabled/disabled in the configuration file (like web.config)
13) Code coverage is more than 95% [Efficiency]
Code coverage is as important as the unit test cases passing. 95% of the code is covered (which means 95% code is actually tested via unit test cases).
14) On-demand resource delivery[Fast]
Resources are fetched and delivered only on demand. Necessary options are available for dealing with huge data such as paginations, etc.
15) No warnings / console logs [Data safety]
No compiler warnings arise while running the application. Logs that are used while developing are cleared and none of the application information (especially the sensitive ones) are written in the browser console.
16) Legal usage of third-party tools/libraries[Licencing]
External libraries are used only if proven necessary for the application. If there are third-party tools or libraries used, then the licenses and legal usages are verified and complaint.