Things everyone should do: Code Review

The most significant thing that makes Google’s code so good is simple - code review (hereinafter CR). Google is not the only company using CR. Everyone knows that this is a good idea and a lot of developers do it. But I have not seen any other big company in which CR would have been so competently implemented. In Google, not a single line of code goes into production until it receives a positive CR rating.

You should pay attention to CR. This is a prerequisite for the development of any software at a serious level. CR should be used not only for the code that goes into production, but generally for everything. This is not such a big difference in time as you think, but the difference in quality is huge.
')
What do you get from using CR?

The obvious: one head is good, but two is better. At least one more person will review your code before it is tested in real conditions (bugs). This is the most frequently mentioned profit from CR. But my experience says that this is just the least valuable. Colleagues do find errors in your code, but the vast majority of such errors found in the process of code review are, frankly, trivial things that the author of the code will find in a few minutes. The bugs that really need to spend time are not caught by CR.

The biggest benefit from code review is social. If you are a programmer and you know that your colleagues will definitely look at your code, you think differently. You will write neat, well-documented and organized code, because you know that people whose opinion on the code is important to you will carefully check the code sent for review. Without CR, you also know that other people will look at your code. But you understand that this will not happen immediately, so this fact does not have such a subconscious effect.

Another big advantage is the spread of knowledge. In many teams, each individual programmer has a piece of code in the project for which he is responsible, and this programmer is focused on this particular code. While colleagues do not break something related to this code, they do not pay attention to it. A side effect of this is that for each component there is only one person who fully understands how it works. If this person does not meet the deadlines or - God forbid - leaves the company, there will be no one left who is familiar with the component code. With CR there will be at least two people who are familiar with the code - the author and the reviewer. Reviewer does not know the code as the author knows it, but he is familiar with the structure and architecture of the code, which is very significant.

Of course, it is not easy. From my experience I can say that it takes time to establish a good CR process. I know a few pitfalls that can cause problems. While these stones periodically crawl out, CR can remain at a bad level, be a burden and prevent the adoption of a daily workflow.

The main rule is that the goal of CR is to find problems in the code before it is committed, i.e. right. The most common mistake in CR for newbies is to check the code, based on how he would do it. Usually there are several ways to solve the same thing. In addition, there are a million ways to write your plan in the form of code. The task of the reviewer is not to make the written code look like it is written, no - it will never be like that. The task of the reviewer is to make sure that the code written by the author is correct. When this rule is violated, you will eventually come to emotions and experiences that are not what you are striving for.

The fact is that this is a completely natural mistake. If you are a programmer when you look at a problem in code, you can immediately see the way to solve it and you think that what you saw and there is a solution. But this is not the case, and to become a good reviewer you need to understand this.

The second most important caveat is that people feel the need to say something. You know that the author has spent enough time and effort working on the code and it seems like you have to express your opinion.

No, you should not.

It’s quite normal to just say “Looks good.” If you are constantly trying to find something to criticize, then all you have to do is undermine your authority and break the relationship in the team. When you criticize the code just to find something to say, then people whose code you check will understand that you write only to write something and your comments will not be taken seriously.



The third is speed. It is not necessary to run headlong to check the code just sent for review, but on the other hand, you need to do it quickly. Your colleagues are waiting for you. If you and your colleagues do not devote time to making CR, and done quickly, then CR can cause people to be dissatisfied with the team. It may seem that this is a focus switch - take on the CR. This is not entirely true. No need to drop everything at the moment when the new code is sent for review. But within a few hours you will definitely pause for something to drink, go to the toilet or just walk around. When you come back from a pause, pay attention to CR. If you take it into habit, then no one in the team will wait your review for a long time and it will not cause any discomfort in the work.

Additional materials:

• How Google Does Code Review
• Code Review Like You Mean It