Practical experience in peer code review

Statement: This article was first published on bole online after I translated it. Please mark the original position for any form of reprinting.

Millions of years ago, ape rose from the tree and evolved into a human being.

From a similar perspective, we look at code review: It seems like something that can separate humans from the beast in the broad field of software development.

However, I sometimes hear the following comments from our team members:

• "Code review for this project is a waste of time ."
• "I don't have time to review the code ."
• "The release of my project has been postponed because my weak colleague has not made any review ."
• "Can you believe my colleagues want me to change something in the code? Please explain to them: If my initial elegant code is changed in any way, it means that the subtle balance of the universe will be damaged ."

Why do we need code review?

First, let's remember why we need code reviews. One of the most important goals for any professional software developer is to continuously improve the quality of their work. Even if your team is a good programmer, you cannot distinguish yourself from a competent free practitioner unless you can work as a team. Code review is one of the most important ways to achieve this goal. In particular, they:

• Give you the second pair of eyes to find the flaws and better ways to do something.
• Make sure at least one other person is familiar with your code.
• Train New Employees by displaying more experienced developer code.
• Promote knowledge sharing by presenting good ideas and practices to reviewers and reviewers.
• Developers are encouraged to do their best in their work because they know that their code will be reviewed by one of their colleagues in the future.

Complete and in-depth review

However, these goals cannot be achieved without putting a certain amount of time and effort into the review. You can only scroll through the patch to make sure that the indentation is correct and that all variables are spelled by camels. This does not constitute a thorough review. Inspired by the industry, you can also consider Pair programming. This is a very popular practice, but it also adds 100% additional overhead for all development time to serve as a benchmark for code review. You may spend a lot of time in code review, but the overall engineering time is still much less than the Pair programming.

I think the time spent on code review should be about 25% of the original development time. For example, if a developer has implemented a small project in two days, the reviewer should spend about four hours reviewing it.

Of course, the amount of time spent on review is not the most important, as long as the review can be completed accurately. Specifically, you must be able to understand the code you are reviewing. This not only means that you only need to understand the language syntax used by the code, but also means that you must understand how the code adapts to a larger application environment, component, or library. If you do not grasp the full meaning of each line of code, your review is not very valuable. This is why a good review cannot be completed very quickly: it takes time to investigate the different code paths that trigger a given function and ensure that the third-party APIs can be used properly (including any edge conditions ), and so on.

In addition to looking for flaws or other issues in your reviewed code, you should also ensure that:

• Includes all necessary tests.
• Appropriate design documents have been written.

Developers who are even good at writing tests and documents do not always remember to update the code as soon as the code is changed. Minor adjustments from code reviewers when appropriate are crucial to ensuring that the Code does not deteriorate over time.

Prevent code review overload

If your team enforces code review, this is risky, because your code review may be stuck in the backlog until it is unmanageable. If you do not perform any review within two weeks, you can easily spend a few days to complete it. But this also means that when you finally decide to deal with them, your own development work will be stranded unexpectedly. This also makes it more difficult to do a good job of review, because the correct code review requires strong and continuous mental work, and it is difficult to maintain it for several days.

Therefore, developers should do their best to clear their review backlog every day. One way is to solve the review first thing in the morning. Complete all the excellent review work before starting your own development work. You can prevent future review from being out of control. Some prefer review before or after lunch break or after the end of a day. No matter when you do these things, you can avoid code review as a regular routine task rather than as a distracting task:

• You do not have time to handle your review backlog.
• The release of the Project is delayed because your review is not completed.
• Make some reviews that are no longer relevant, because the Code has changed a lot during this period.
• The final completion of the review was poor because the review was handled at the last minute.

Write code that is easy to review

The review backlog cannot be managed by all reviewers. If my colleagues spend a week to add code to a large project, the patches they release will be hard to review, because there is too much work to be done in a phase, the purpose of the code and the underlying architecture system will be hard to understand.

This is one of the many reasons for splitting your work into manageable units. We use the scrum management method, so the appropriate unit is the focus for us. By working together, we use units to organize our work and submit reviews that are only related to a specific unit that we are conducting, we can write code that is easier to review. Your team may use another management method, but the principles are the same.

In order to write code that is easy to review, there are other necessary conditions. It is reasonable to discuss some difficult architectural decisions in advance to meet the requirements of reviewers. This will make it easier for reviewers to understand your code, as they will know what you are trying to do and how you plan to do it. This also helps avoid a situation where, after the reviewer proposes a different and better method, you must rewrite your large code section.

The project architecture should be described in detail in your design documents. This is important in any case because it allows a new project member to quickly catch up with the progress and understand the existing code library. It can also help reviewers do their jobs better, which is another benefit. Unit tests also help to explain to reviewers how components should be used.

If your patch contains third-party code, submit it separately. For example, when jquery's 9000 lines of code are inserted into the middle of the Code, it is difficult to review the code.

One of the most important steps to write code that is easy to review is to add comments to your code review section. This means you can view the review section by yourself and add comments wherever you feel helpful to the reviewer's understanding of the Code. I found that such comments only take a relatively small amount of time (usually only a few minutes), but can play a huge role, so that the Code review can be completed faster and better. Of course, code comments also have many advantages that should be used in appropriate places, but it is generally wise to review comments. Finally, it can be said to be a reward. Research shows that when developers re-read and comment on the code, they actually find many flaws in their own code.

Huge code refactoring

Sometimes it is necessary to refactor a code library that can affect many components. For a large application, this process may take several days (or even longer) and cause a large patch. In these cases, a standard code review may be impractical.

The best solution is to incrementally refactor the code. Find a change point that can achieve your purpose within a reasonable scope of the work code library. Once the review is modified, the review is passed and the next change is made until the reconstruction is completed. This method may not work every time, but with ideas and plans, it is usually practical to avoid huge patches during refactoring. Refactoring code like this may take more time for developers, but it also produces better code quality and easier review work.

If the Code cannot be incrementally restructured (this may be about writing and organizing the source code ), one possible solution is to use Pair programming instead of code review for refactoring.

Dispute resolution

Your team is undoubtedly composed of a group of smart professionals. When you disagree with a fixed coding issue, it is basically controversial. As a developer, stay open and be prepared to compromise when your reviewers prefer a different approach. Do not take a proprietary attitude towards your code, or take personal review comments. If someone thinks you should refactor some repeated code into a reusable function, this does not mean that you are not an attractive, outstanding, and attractive person.

As a reviewer, you must be witty. Before changing your suggestion, consider whether your suggestion is really better or just your personal style. If the battlefield you choose is concentrated in areas with significant improvements in source code, you will be able to achieve more success. Say something like "think about ...... It may be worthwhile "or" someone suggested ......" It is more suitable than "even my pet hamster can write a sorting algorithm that is more efficient than this ".

If you cannot reach an intermediate position (that is, neither party is willing to compromise), invite a third-party developer from both parties to come and have a look and give them some opinions and suggestions.

Practical experience in peer code review