- 是否存在测试，它们是否可以被理解？比如，至少达到你满意的代码覆盖(code coverage)。
Peer code reviews are the single biggest thing you can do to improve your code.
- Does the code work? Does it perform its intended function, the logic is correct etc.
- Is all the code easily understood?
- Does it conform to your agreed coding conventions? These will usually cover location of braces, variable and function names, line length, indentations, formatting, and comments.
- Is there any redundant or duplicate code?
- Is the code as modular as possible?
- Can any global variables be replaced?
- Is there any commented out code?
- Do loops have a set length and correct termination conditions?
- Can any of the code be replaced with library functions?
- Can any logging or debugging code be removed?
- Are all data inputs checked (for the correct type, length, format, and range) and encoded?
- Where third-party utilities are used, are returning errors being caught?
- Are output values checked and encoded?
- Are invalid parameter values handled?
- Do comments exist and describe the intent of the code?
- Are all functions commented?
- Is any unusual behavior or edge-case handling described?
- Is the use and function of third-party libraries documented?
- Are data structures and units of measurement explained?
- Is there any incomplete code? If so, should it be removed or flagged with a suitable marker like ‘TODO’?
- Is the code testable? i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.
- Do tests exist and are they comprehensive? i.e. has at least your agreed on code coverage.
- Do unit tests actually test that the code is performing the intended functionality?
- Are arrays checked for ‘out-of-bound’ errors?
- Could any test code be replaced with the use of an existing API?
Optimize Your Checklist
Using the checklist as a starting point, you should optimize it for your specific use-case. A great way to do this is to get your team to note the issues that arise during code reviews for a short time. With this data, you’ll be able to identify your team’s common mistakes, which you can then build into a custom checklist. Be sure to remove any items that don’t come up (you may wish to keep rarely occurring, yet critical items such as security related issues).
Get Buy-in and Keep It Up To Date
As a general rule, any items on the checklist should be specific and, if possible, something you can make a binary decision about. This helps to avoid inconsistency in judgments. It is also a good idea to share the list with your team and get their agreement on its content. Make sure to review the checklist periodically too, to check that each item is still relevant.
Armed with a great checklist, you can raise the number of defects you detect during code reviews. This will help you to drive up coding standards and avoid inconsistent code review quality.
Review the right things, let tools to do the rest
You don’t need to argue over code style and formatting issues. There are plenty of tools which can consistently highlight those things. Ensuring that the code is correct, understandable and maintainable is what’s important. Sure, style and formatting form part of that, but you should let the tool be the one to point out those things.
Everyone should code review
Some people are better at it than others. The more experienced may well spot more bugs, and that’s important. But more important is maintaining a positive attitude to code review in general and that means avoiding any ‘Us vs. Them’ attitude, or making reviewing code burdensome for someone.
Review all code
No code is too short or too simple. If you review everything then nothing gets missed. What’s more, it makes it part of the process, a habit and not an after thought.
Adopt a positive attitude
This is just as important for reviewers as well as submitters. Code reviews are not the time to get all alpha and exert your coding prowess. Nor do you need to get defensive. Go in to it with a positive attitude of constructive criticism and you can build trust around the process.
Code review often and for short sessions
The effectiveness of your reviews decreases after around an hour. So putting off reviews and doing them in one almighty session doesn’t help anybody. Set aside time throughout your day to coincide with breaks, so as not to disrupt your own flow and help form a habit. Your colleagues will thank you for it. Waiting can be frustrating and they can resolve issues quicker whilst the code is still fresh in their heads.
It’s OK to say “It’s all good”
Don’t get picky, you don’t have to find an issue in every review.
Use a checklist
Code review checklists ensure consistency – they make sure everyone is covering what’s important and common mistakes.
Keep the code short
Beyond 200 lines and the effectiveness of a review drops significantly. By the time you’re at more than 400 they become almost pointless.
Link to any related tickets, or the spec. There are code review tools like Kiln that can help with that. Provide short, but useful commit messages and plenty of comments throughout your code. It’ll help the reviewer and you’ll get fewer issues coming back.