In our
article on effective code review, we recommended using a checklist. Checklists (checklists) are a great thing in a review: they ensure that the review actually passes through your team. They also help to identify and solve common difficulties.
A study by the Software Engineering Institute shows that programmers make 15–20 common mistakes. By adding such errors to the checklist, you can be sure that you will notice them when they appear and help you get rid of them for a long time.
So that you can start from something, here is a list of typical points:
Checklist CodeReview
General
- Does the code work? Does he fulfill his direct duties, is the logic correct, etc.
- Is the code easy to understand?
- Does the code match your style of writing code? This usually refers to the arrangement of brackets, variable and function names, line lengths, indents, formatting, and comments.
- Is there a redundant or duplicate code in the review?
- Is the code as independent as possible?
- Can I get rid of global variables or move them?
- Is there any commented out code?
- Do cycles have a fixed length and correct completion conditions?
- Can something in the code be replaced by library functions?
- Can a part of the code intended for logging or debugging be removed?
Security
- Are all input data validated (for correct type, length, format, range) and encoded?
- Are errors handled when using third-party utilities?
- Are the outputs checked and encoded ( note: for example, from XSS )?
- Are invalid parameter values ​​processed?
Documentation
- Are there any comments? Do they reveal the meaning of the code?
- Are all features commented on?
- Is there any unusual behavior or description of borderline cases?
- Is the use and operation of third-party libraries documented?
- Are all data structures and units described?
- Is there an unfinished code? If so, should it be removed or marked with a “TODO” marker?
Testing
- Is the code testable? For example, it should not contain too many dependencies or hide them, test frameworks should be able to use code methods, etc.
- Are there any tests, and if so, are they sufficient? For example, they cover the code in the right measure.
- Are unit tests actually verifying that the code provides the required functionality?
- Are all arrays checked for "going beyond the boundaries"?
- Can any testing code be replaced using an existing API?
You may also want to add to the checklist any items that are specific to your language and may cause problems.
Checklist deliberately does not cover all possible problems. You do not need a checklist that is so long that no one uses it. It is best to cover only common questions.
')
Optimize your checklist.
This checklist can be used as a starting point: you need to optimize it for your specific scenarios and problems. A great way to do this is to mark the questions that arise on your team during the review of the code for a short time. With this information you can find the most common mistakes of the team in order to then enter them into your checklist.
Do not be afraid to throw out from the checklist those elements that do not suit you (you may want to leave rare, but still critical points - for example, related to security).
Start using and keep up to date
As a rule, any checklist items should be specific and, if possible, give you an unequivocal answer to your question. This helps to avoid inconsistency of judgment.
Another good idea is to share the list with your team and get their acceptance of its contents. Periodically look through the checklist itself to check that every item in it is still relevant.
Armed with a good checklist, you can increase the number of defects detected during the review of the code. This will allow you to improve the coding standard and avoid poor quality reviews.
To learn more about the review code, you can read the video on our
page .