Why use a code review checklist?
- Key benefits of a code review checklist
- Preliminary steps
- Architecture review
- Code quality review
- Test coverage and quality
- Security
Why use a code review checklist?
A code review checklist ensures consistency across development teams, supporting both seasoned developers and new members unfamiliar with the best practices of their programming frameworks.
Key benefits of a code review checklist
- Identify issues promptly: Pinpointing flaws early is crucial for software enhancement. Checklists enable quick detection of critical issues, preventing them from becoming larger problems.
- Uphold best practices: A checklist helps teams adhere to best practices, reducing technical debt and enhancing security, performance, and maintainability. This keeps everyone aligned with strategic objectives.
- Boost collaboration: By establishing clear expectations for code discussions, checklists improve communication, expedite issue resolution, and raise software quality.
- Streamline processes: Standardizing reviews reduces time spent on discussions about code changes. A clear checklist allows for faster issue spotting and streamlined resolutions, enhancing productivity.
Key elements of code review: manual and automated
A comprehensive code review should consist of two parts – manual and automatic verification. A combination of both helps achieve the highest quality and value from code reviews.
During the manual review, the following general aspects are checked:
- Architecture
- Code style and cleanliness
- Class and variable names
- Unit tests
- Documentation
When performing manual verification, it is necessary to rely on code review best practices.
Automatic review uses tools that help check the system for vulnerabilities, outdated libraries, and unused code by analyzing large amounts of data. All metrics that are found using the program must be carefully described.
The following tools can be used for automatic verification:
- Static code analyzers to find potential bugs and security vulnerabilities in code
- Code coverage tools to measure how much code is executed by a test suite
- Linters to spot deviation from coding style rules
- Test automation tools to run tests automatically
Remember, while automation tools are crucial in supporting and enhancing professional code reviews, they cannot replace the critical thinking, judgment, and domain expertise that human reviewers bring to the process. Now let’s get to the actual code review template.
Our comprehensive code review checklist
Preliminary steps
There are several preliminary steps you should take before diving into the code review itself. They help ensure a more focused, productive, and collaborative code review experience for everyone involved.
Project Requirements and Objectives:
- Read project documentation, including requirements, specifications, and design documents
- Understand the primary goals of the code review – to identify security issues, improve code quality, ensure adherence to coding standards, or something else?
- Find out if there are specific areas of focus or concerns for the review
Project Scope:
- Determine if the review will focus on specific modules, features, or functionalities within the codebase
- Identify areas where problems have occurred in the past or where technical debt is known to exist
- For projects in regulated industries, ensure that relevant sections of the code are reviewed for compliance/li>
- In the case of security audits, identify components that handle sensitive data or are critical to system security
Environment Setup:
- Ensure you have access to the codebase, testing tools, and any relevant documentation
- Choose appropriate tools for your review process, like code linters, static analysis tools, or integrated development environments (IDEs)
- If applicable, ensure you can run and test the code effectively
Architecture review
Architecture review is an integral part of our code review checklist. It assesses how the code adheres to the overall system’s design principles and long-term maintainability. Early detection of architectural issues prevents costly rework later in the development process. It also ensures that the system can handle growth and perform efficiently under varying loads.
Alignment with Architectural Decisions:
- Check if the code adheres to the documented system architecture in specs
- See if the chosen architectural patterns are appropriate for the problem domain and system requirements
Separation of Concerns and Modularity:
- Check if the code is split into multiple layers, such as business logic, data access, and presentation if it’s required
- Check if the code is split into respective files, for example for frontend – HTML, JavaScript, and CSS
- Examine if the modules are well-defined, cohesive, and loosely coupled
- Ensure there are no signs of spaghetti code or tightly coupled components
Object-Oriented Analysis and Design Principles:
- Check if the single responsibility principle (SRP) is followed: no more than one responsibility should be placed into a single class or function
- Ensure the code adheres to the open-closed principle (OCP): while adding new functionality, it should be written in new classes and functions; existing code should not be modified
- Inspect if the Liskov substitution principle (LSP) is observed: the child class should not change the behavior (meaning) of the parent class; the child class can be used as a substitute for a base class
- Examine interface segregation: Lengthy interfaces should be split into smaller ones based on functionality. The interface should not contain any dependencies (parameters) which are not required for the expected functionality
- Examine dependency injection: dependencies shouldn’t be hardcoded but injected
Example of a SRP violation:
public class CustomerManager < public void AddCustomer(Customer customer) < /* Implementation */ >public void UpdateCustomer(Customer customer) < /* Implementation */ >public Customer GetCustomer(int id) < /* Implementation */ >public void SendWelcomeEmail(Customer customer) < /* Implementation */ >public void GenerateCustomerReport() < /* Implementation */ >>
- Review if tables are properly normalized to eliminate data redundancy while considering denormalization where performance is critical
- Check for the implementation of database constraints, transactions, and rollback strategies to handle errors
- Review the strategies for data backup and recovery to ensure data durability
- Assess if commonly queried fields are indexed to speed up search operations
- Analyze how sensitive data is handled, including encryption and access control mechanisms
Integration and Interoperability:
- Assess the design, documentation, and management of APIs, ensuring they’re well-defined and facilitate secure, reliable communication
- Evaluate the integration with external services and management of external dependencies
- When applicable, verify if the system supports standard data formats for interoperability, like JSON for web services
- Assess the use of middleware or integration tools for connecting disparate systems
Scalability and Performance:
- Check if the architecture can handle expected future growth in terms of users, data, and transactions
- Check if if the architecture can scale both vertically (adding more resources to a single server) and horizontally (adding more servers)
- Verify if resource-hungry operations are moved into separate services and called via queue/other mechanism
- Check if it’s possible to move separate services to different server instances and if it’s possible to run multiple instances of service
- Review performance optimization strategies, including caching, load balancing, and asynchronous processing, to ensure the system can handle peak loads efficiently
- Examine fallback and redundancy mechanisms designed to maintain system availability and performance under adverse conditions
Security:
- Analyze the app’s security framework, ensuring that it incorporates a layered defense strategy to protect against various types of threats
- Review authentication, authorization, and auditing mechanisms to confirm they are robust and comply with security best practices
- Inspect encryption protocols and security measures applied to data in transit and at rest, verifying their adequacy in safeguarding sensitive information
Code quality review
Next on our code review checklist is evaluating the code quality for readability, maintainability, reusability, reliability, and extensibility. Clear and well-formatted code is easier to understand and maintain, leading to fewer bugs, faster development, and reduced technical debt. Also, a consistent coding style fosters better collaboration among team members, as everyone can easily work with each other’s code.
Methods and Functions:
- Check if the functions’ names have the form “expressive verb + object” so it’s concise and easy to understand
- Inspect if the functions’ names describe all the actions they perform
- Evaluate if the function is too big and whether it should be refactored into smaller functions
- Analyze if the function is doing only one thing but doing it well (single responsibility principle)
Variables:
- Verify if each variable has one and only one goal
- Check if variables are named according to the convention, such as camelCase, kebab-case, and snake_case
- Ensure variables’ names describe the represented entity fully and accurately
- Check if variables’ names strike a balance between providing enough information and keeping the names concise
- Ensure variables’ names make it easy to understand what kind of data they keep
- Check if there are any magic numbers or strings
Example of magic strings:
if (message == "Insufficient funds") < // Handle insufficient funds error >else if (message == "Invalid product ID") < // Handle invalid product ID error >// . more logic based on string comparisons
- Check if complex checks are converted to calls of logical functions
- See if the most likely cases are checked first. Are all options considered?
- Verify if there are any functions that contain many conditional expressions. Is there a good reason for not redesigning them?
Recursion:
- Check if the recursive function contains an exit point
- See if the base case is easy to understand and recognize. Complex base cases can obscure the overall logic
- Check if the recursion depth matches the constraints imposed by the size of the program stack
Class/Structure Maintainability:
- Check if all data structures have a central purpose
- Ensure all data structures are well-named and their names describe their central purpose
- Check if all data structures are independent of other classes and if they are loosely coupled
- Inspect if all data structures collaborate with other classes only when it’s absolutely necessary
- Check if all data structures hide their implementation details from other data structures as much as it’s necessary
Reusability:
- Ensure the code follows the “Don’t repeat yourself” (DRY) principle: every piece of knowledge must have a single, unambiguous, authoritative representation within a program
- Examine if there is any duplicated code that can be refactored into a common method or class
- Check if there are functions or methods that perform similar tasks which can be consolidated
Example of functions that can be consolidated into a single method with an additional parameter:
public class CustomerService < public Customer GetCustomerById(int id) < // Logic to retrieve customer from database by ID return customer; >public Customer GetCustomerByEmail(string email) < // Logic to retrieve customer from database by email return customer; >>
- Check if the error handling technique is defined, specifying the format of errors (messages, status codes) and where to send errors (console, file, AWS S3)
- Check if errors are being ignored anywhere. For example, an empty “catch” block means the error is ignored, which makes debugging difficult
- Examine if the error-handling code is centralized in a single module that can be replaced if necessary
- Verify if too much information is returned to the user, and no sensitive information like system paths, database details, or internal error codes is exposed
- Assess the implementation of logging levels, such as DEBUG, INFO, WARN, ERROR, FATAL
- Assess the policies for log rotation and archiving to manage log file sizes and retain historical data
Example of code that hinders effective debugging:
public decimal CalculateAverage(List numbers) < try < int sum = 0; foreach (var number in numbers) < sum += number; >return sum / numbers.Count; > catch (Exception) < // Log the error somewhere (omitted for brevity) return 0; >>
- Analyze if a particular library is really needed
- Check if the library is distributed under an open-source license
- Inspect if there are any known vulnerabilities in the library
- Check if there are some not-known libraries used, especially instead of popular ones and well-supported by the community
- Check if the library maintainers and community are active. Is it easy to contact them in case of issues?
- Examine if the project uses different frontend libraries in different sections
Comments and Documentation:
- Ensure comments are meaningful, concise, and explain non-obvious code only
- Inspect the documentation for completeness and clarity, especially for parts of the code that are complex or pivotal
- Confirm that all comments and documentation reflect the most current version of the codebase, taking into account any recent changes or updates
Code Simplification and Refactoring:
- Spot opportunities for refactoring or simplifying the code to reduce complexity without losing functionality
- Promote the removal of superfluous or obsolete code segments, making the codebase leaner and more efficient
- Support the use of recognized design patterns and frameworks where suitable for a shared understanding among developers and adherence to industry best practices
Test coverage and quality
Test coverage ensures your testing efforts cover as many aspects of your app as possible. This includes code coverage, requirements, functionality, business logic, user scenarios, performance, and security. Uncovered code could hide critical bugs. But remember, high code coverage doesn’t equal high quality. Look at your tests’ design, execution, and effectiveness for true software excellence.
Test Coverage:
- Ensure the test suite covers all the functional requirements specified in the documentation
- Verify that the tests cover boundary conditions and edge cases
- Assess the balance between unit tests and integration tests: there should be a sufficient number of unit tests for individual components and integration tests to ensure that these components work together
Documentation and Maintainability:
- Check if each test has a clear description of what it is testing and the expected outcome
- Review the structure and organization of the test code for ease of maintenance: the use of test fixtures, setup/teardown methods, and data-driven tests
- Ensure that any documentation associated with the tests, like test plans or test case descriptions, is current and matches the actual tests
Test Reliability and Stability:
- Check if tests produce consistent results and aren’t flaky
- Examine how tests handle external systems or dependencies like databases or APIs
- Check if test failures clearly indicate the problem, returning meaningful error messages and logs that pinpoint the source of failure
Performance and Efficiency:
- Review the total time taken to run the test suite. Long-running tests could be optimized or run in parallel
- Assess if the tests are efficient in terms of memory and CPU usage
- Ensure the test suite can handle increased loads, which is particularly important for performance tests
Security
Imagine building a house without checking for structural flaws. Similarly, insecure code exposes your app to attacks, jeopardizing users’ data, system integrity, and reputation. That’s why our code review guidelines also include a security check.
Authentication and Authorization:
- Examine if passwords are securely stored (hashed and salted)
- Ensure strong password policies are enforced, including password length and complexity
- If multi-factor authentication is required for privileged access, check if it’s there
- Check if authorization checks are present for all sensitive actions
- Verify if least-privilege principles are followed (granting only necessary permissions)
Input Validation and Sanitization:
- Check if all user input, such as type, length, and allowed characters, is validated
- Examine if data sanitization techniques, escaping, or encoding are used to prevent injection attacks
- Check if stored procedures are used for database interactions to avoid manual SQL construction
Data Security:
- Check if sensitive data elements are encrypted at rest and in transit using strong algorithms like AES and TLS
- Ensure secure key management practices are followed, such as using hardware security modules for storage and rotating keys regularly
- Ensure data access is restricted to authorized users and processes
- Verify data disposal procedures, ensuring data is overwritten to prevent unauthorized recovery
Session Management:
- Analyze the security of session IDs to prevent session hijacking. Are they random and unguessable?
- Ensure sessions are invalidated after inactivity or logout
- Check if CSRF (Cross-Site Request Forgery) tokens are implemented to prevent unauthorized actions
- Check if secure communication protocols like HTTPS are used
Error Handling and Logging:
- Check if generic error messages are provided instead of detailed stack traces exposing internal details
- Ensure logs are securely stored and monitored for suspicious activity, such as failed login attempts or unusual access patterns
- Verify if logs avoid storing sensitive data like passwords or credit card numbers
Secure Coding Practices:
- Check if security best practices like the OWASP Top 10 are followed
- Analyze if memory management practices are secure to prevent buffer overflows and other memory-related exploits
- Check if cryptographically secure random number generators are employed for critical tasks like password generation and session tokens
How to improve the code review process
After mastering what to check in a code review, it’s time to refine the process itself. Here are several strategies to elevate your code review quality:
- Implement a comprehensive style guide: Beyond settling style debates about whitespace or naming, a thorough organization-wide code style guide should encompass best practices for utilizing features of the programming language effectively
- Set deadlines: Create clear guidelines for the expected turnaround times based on code length and complexity, such as aiming for one-day reviews for standard updates and allowing more time for extensive changes
- Encourage insightful dialogue: Shift from merely commenting on code to asking the author about their choices. Inquire about the rationale behind specific formats or decisions to foster a constructive dialogue
- Promote team unity in feedback: Phrase critiques in a collective context, using statements like “We prefer this method because it enhances readability.” Steer clear of accusatory feedback, which can help in maintaining a positive team spirit
- Base feedback on objective principles: Provide feedback rooted in established coding principles or frameworks rather than personal preferences. This approach not only helps in maintaining consistency but also aids in creating an educational environment for developers
- Concentrate on high-impact improvements: While reviewing, prioritize changes that significantly enhance the project’s value. Focus on critical improvements over perfection, considering the project’s time and scope constraints
Our experience with code reviews
For Redwerk, code reviews are a very natural thing to do. We peer-review the code we write while developing software solutions for our clients. But we’re equally happy to look at external code and share our expertise to improve those products and help out fellow developers. Here’s a glimpse into our latest code review projects.
Project Science
Project Science is IT quote management software developed by Complete Network, a leading provider of managed network and IT support services in the US. Complete Network hired Redwerk to perform a code review of the Project Science backend API.
We reviewed the architecture, database structure, and code quality and flagged 40 critical issues. We spotted easy opportunities to increase the app’s performance by using Django caching, database query caching, and Python speed-up tools. Our Python developers reported 50 instances of unused code, empty files, unused functions, the overuse of Python reserved words, and other readability and reusability issues.
With our help, Complete Network has managed to increase its code maintainability by 80% and adopted healthy coding practices to prevent similar issues in the future.
Site Compass
Site Compass is a network mapping app developed by Reivernet Group, a US-based network system integrator, managing complex data networks for the hospitality, education, and government sectors. It’s built in C# using Uno Platform.
Reivernet Group turned to Redwerk to receive an impartial audit of the Site Compass app and prepare for the official release. Our scope of work included reviewing the architecture, database, backend quality, code coverage, and security.
We reported examples of tight coupling, classes with overly high depth of inheritance, methods with too many parameters, overly big types leading to the god object phenomenon, and several other issues. Thanks to our audit, Reivernet Group was able to fix the critical problems before taking the product to market, resulting in 90% code readability and maintainability.
Still have some code review questions? Tell us about your needs, and we’ll arrange a free consultation for you.
FAQ
What distinguishes a code review from a code checker?
Code reviews involve manual examination by peers to assess code quality and intent, while code checkers are automated tools that scan for specific issues like vulnerabilities or style violations.
Which tools can enhance the code review process?
Tools such as static code analyzers, linters, and version control platforms streamline code reviews by automating error detection and facilitating collaboration.
What benchmarks and metrics are important in code reviews?
Key metrics include defect density, code complexity, and adherence to coding standards, which help gauge the effectiveness of the review process and code quality.
How does a style guide contribute to effective code reviews?
A style guide standardizes coding practices across the team, reducing subjective debates and focusing reviews on substantial issues rather than stylistic preferences.
What strategies can make code reviews more effective?
Effective strategies include setting clear review objectives, using code review checklists to ensure thoroughness, and fostering a culture of constructive feedback, all of which contribute significantly to the overall improvement of code quality.
