What Do Code Reviewers Look For?
Code reviews should look at:
- Design: Is the code well-designed and appropriate for your system?
- Functionality: Does the code behave as the author likely intended? Is the way the code behaves good for its users?
- Complexity: Could the code be made simpler? Would another developer be able to easily understand and use this code when they come across it in the future?
- Tests: Does the code have correct and well-designed automated tests?
- Naming: Did the developer choose clear names for variables, classes, methods, etc.?
- Comments: Are the comments clear and useful?
- Style: Does the code follow our style guides?
- Documentation: Did the developer also update relevant documentation?
What do peer reviewers look for?
Feature Completion
The reviewer will make sure that the code meets the requirements, pointing out if something has been left out or has been done without asking the client.
Potential Side Effects
The reviewer will check to see whether the changed code causes any issues in other features.
Readability and Maintenance
The reviewer will make sure the code is readable and is not too complicated for someone completely new to the project. Model and variable names should be immediately obvious (again, even to new developers) and as short as possible without using abbreviations.
Consistency
Conducting peer reviews is the best approach for achieving consistency across all company projects. Define a code style with the team and then stick to it.
Performance
The reviewer will assess whether code that will be executed more often (or the most critical functionalities) can be optimized.
Exception Handling
The reviewer will make sure bad inputs and exceptions are handled in the way that was pre-defined by the team (it must be visible/accessible to everyone).
Simplicity
The reviewer will assess whether there are any simpler or more elegant alternatives available.
Reuse of Existing Code
The reviewer will check to see if the functionality can be implemented using some of the existing code. Code has to be aggressively “DRYed” (as in, Don’t Repeat Yourself) during development.
Test Cases
Finally, the reviewer will ensure the presence of enough test cases to go through all the possible execution paths. All tests have to pass before the code can be merged into the shared repository.
No comments:
Post a Comment