First, let’s take a look at the “death triangle” of a project that scares countless technical leaders. Business pressure leads to a decline in code quality, which leads to a decline in development efficiency, which in turn increases business pressure, and finally leads to a huge amount of business pressure and even a project failure. How to crack it? There are many methods, such as streamlining business requirements, increasing development staff, upgrading technical architecture, and so on. In many cases, multiple methods need to be implemented at the same time. If you eliminate any part of the “death” triangle, you can end the vicious circle, or even reverse the virtuous circle.

The first target of a Code Revew, or CR, is obviously “bad Code.” The best time to avoid “bad code” is when you’re writing code, followed by when you’re reviewing it. IBM’s Orbit project, with 500,000 lines of code, used 11 levels of code review (which includes code reviews), and the result was that the project was delivered ahead of schedule with only 1% of the usual expected errors. A study of a 200-strong group at AT&T reported a 14 percent increase in productivity and a 90 percent reduction in defects after the introduction of code reviews. So what exactly is a code review? How do code reviews take place? Keep reading.

1 CR disenchantment

I personally believe that code has these levels: 1) compilable, 2) runnable, 3) testable, 4) readable, 5) maintainable, and 6) reusable. CODE that passes automated testing will only reach level 3), and CODE that passes CODE REVIEW will rarely reach level 4) or higher.

– taking

Which of the following 8 statements about CR do you think is correct?

  1. Formalism is a waste of time
  2. CR is the means to ensure the correctness of the program
  3. CR is a means of ensuring code specification
  4. CR is the Leader’s job, it has nothing to do with me
  5. I only read the CR that was pointed at me. The other CR doesn’t concern me
  6. No time for CR, direct Merge
  7. CR must be seen from beginning to end
  8. CR must be done all at once

———————————————————————————————— Think about it for 60 seconds

3… 2… 1… Time is up. What are your answers? I’m sorry, but in my opinion, none of that is true. 1, 4, 5 and 6. CR is like reading a book. Not all books are suitable for accuracy, and not all code needs to be reviewed. 8 is task mentality, CR for CR’s sake, CR’s purpose is not to complete CR, but to improve the quality of the code, and you don’t write code to do everything at once. More controversial is 2 and 3, true, correct and code specification are CR to watch, but that doesn’t mean the CR code criteria to ensure the correctness and also can’t guarantee (CR), ensure correctness is the main means of testing (unit testing, integration testing, test of contract, functional testing, test automation, etc.), Ensuring code specification relies heavily on code specification checking tools (such as checkStyle and PMD).

What exactly is CR? As I see it, CR is essentially a discussion, a serious, professional, asynchronous discussion in written form, where randomness and emotionality are taboo. What is randomness? Uncensored comments. What is emotional? It varies from time to time and from person to person. A high level of CR starts with forgetting yourself.

2. Knowledge: Triple realm of CR

Technical level determines the lower limit of CR, while cognitive height determines the upper limit of CR. So whether CR level is high or not depends ultimately on cognitive level. There are three levels to understand CR: executive level, team level and cultural level.

2.1 executive floor: last night the West wind withered green trees, alone on the high-rise, looking at the end of the world road

The first layer is the implementation layer, which, as the name implies, is how CR is understood. The following are six aspects of CR that need to be focused on, with corresponding examples to facilitate understanding.

1) Focus on code specifications. Naming comes first, and a confusing naming often hides a design flaw. Other issues, such as whitespace characters, line breaks, and comments, can also affect the readability and understandability of code.

2) Avoid duplicate code. First rule of programming, Don’t repeat yourself. Duplicate code is the worst of all evils. Duplicate code is punishable by death.

3) Reduce cyclomatic complexity. What is cyclomatic complexity? This is simply the number of if/case/for/while occurrences in your code. The higher the cyclomatic complexity, the higher the BUG rate. If a method has cyclomatic complexity of 3 or more, CR should be done twice.

4) Focus on performance. Performance issues, while rare, can be a major problem when they occur. Keep an eye out when you see cycles in CR.

5) Focus on distributed transactions. Scenarios involving remote service invocations, or cross-library updates, should consider whether distributed transactions are a problem and how to handle them, whether to rely on the framework for strong consistency, log exception data for ultimate consistency, or simply ignore them.

6) Focus on architectural design. Code has code specifications, architecture has architecture specifications. Merge Request (MR) for a new function should not only check the architecture specification, but also review its architecture design, such as whether it conforms to the three principles of clean architecture, dependency free principle, stable dependency principle, and stable abstraction principle.

In addition to online CR, there is a special offline CR method, which skips MR, pulls the code directly, does the whole CR, marks the review comments in the code as TODO or FIXME, and then @ related development for improvement. The biggest benefit of doing this is to avoid being influenced by a single MR and falling into the trap of not seeing the forest for the trees.

2.2 Team level: the dress is gradually expanded, but I do not regret. I am emaciated for Iraq

Next, look at the second layer, how to understand CR from a team perspective. As mentioned above, CR is essentially a discussion. Bacon said, “Reading makes a man complete, and discussion makes a man complete.” What does CR mean for individuals and teams?

  • Improve your awareness: From a personal perspective, when you know that your code will be reviewed by another set of eyes, you will be on your toes when writing code, letting go of the illusion that god knows, I know, you don’t know, and actually writing every line of code.

  • Establish a good development rhythm: From the team’s perspective, CR is the synchronizer of the team. Each person is both the author of his own MR and the reviewer of others’ MR. From MR to CR, and then from CR to MR, it constitutes the most beautiful music of each working day.

  • High-frequency team activities: From a team perspective, SINCE CR is a discussion, it is not just a matter of one person, but a team activity with high frequency, high quality and low cost and very cost-effective.

2.3 cultural layer: the crowd to find him thousands of baidu, suddenly look back, that person is in, the lights dim

Finally, the cultural layer, CR is not only an important component of mentoring culture, but also the daily embodiment of engineer culture.

  • An important component of mentoring culture: senior engineer CR junior engineer code, can give high-frequency, high-quality guidance; Junior engineers can appreciate and learn from senior engineers how to play with code, and take the essence and discard the dregs.

  • The daily embodiment of engineer culture: collaboration, efficiency, initiative and influence, which frequently appear in the engineer culture of major Internet companies, are all closely connected with CR. It is no exaggeration to say that whether engineer culture is good or not depends on whether CR is good or not.

Line 3: Efficient method for CR

After understanding CR, let’s discuss how to efficiently carry out CR. In my opinion, efficient CR first depends on the following objective and subjective conditions.

Objectively, a harmonious engineer culture and a clear code specification are the two cornerstones of efficient CR. A good engineering culture is one in which teams are open to code, open to secrecy, proud of good code, ashamed of bad code, and constantly focused on code quality. The clear code specification, on the one hand, improves the readability of the code, on the other hand, also unified the coding style, greatly reducing the interference of different code styles to the attention of reviewers.

Subjectively, for reviewers, the first thing to do is correct attitude and keep a humble attitude. To err is human, to choose the good and from it, and to change the bad. The second thing to remember is that the review is about code, not people, and every comment you write should be based on objective facts and data. It should be based on facts and data, not personal feelings.

Based on my years of hands-on CR experience and in conjunction with the Google Code Review Developer Guide, I have compiled some best practices for efficient CR for your reference:

  • According to my personal preference, I have fixed several hours for CR every day. My habit is before going out and before going off work. CR takes as much mental energy as coding, if not more, and requires intense concentration. A clear mind and a free environment are the keys to delivering quality reviews.
  • In addition to fixed time periods, task switching is also a good time for CR.
  • Each CR should be controlled within 15 to 30 minutes. If the CR exceeds 30 minutes, take a rest.
  • After receiving MR, judge the nature of MR first. If it is a Bug Fix type MR, review it as soon as possible; if it is a new function MR, wait for the next CR window.
  • From receipt of MR to CR, no more than 1 working day.
  • Understand the background of the problem MR is trying to solve before starting CR.
  • CR is like reading a book: first look at the table of contents (list of files that have changed), then peruse the key sections (code that contains the core business logic), and finally scan the rest of the chapters.
  • If a large number of files are changed, you can open the IDE and switch to the source branch so that you can open the relevant code for reading at any time during CR.
  • If serious problems are found during core code review, CR should be terminated immediately and discussed in person with MR submitter.
  • If MR submitter has any objection to the review, the reviewer should discuss it with the submitter in person and avoid kicking each other in the comments section.
  • Ensure that all reviews are properly addressed before merging code.
  • Remember to like it. CR is more than just a suggestion, and don’t be stingy with praise when you see elegant code.

4 summary

According to a 2019 Stack Overflow survey, more than 70% of programmers consider CR part of their daily work, nearly one-third spend two to three hours a week on CR, and another third spend four to five hours a week on CR. Do the math in your head. Are you a drag or a leader? If you haven’t done CR yet, do it now; If you are already in CR, great, please keep it up. A flower a world, a leaf a bodhi, code has its own universe. CR, let’s go!

5 reference

  • How to complete a high quality CODE REVIEW? (2020 edition)
  • Chen Hao: A few tips in CODE REVIEW
  • Chen Hao: How to make technology from CODE REVIEW
  • How to do an effective code review? I have these suggestions