After coming back in the Spring Festival, I took over the management of a technical team. The team was in charge of the project for the whole year last year and often worked overtime until after 10 PM, so the code quality could be expected without saying much. In a meeting with everyone a few days ago, I proposed to do a code review starting from the next version. One of my classmates asked me, “Won’t that waste a lot of time?” “I tell people,” It will take a lot of time, but not a lot of time. The students in this team have never done code review before, so they plan to share it with you next week and tell you how to do it. So today, I spend some time to sort out the ideas in my head.

First, what is the purpose

Everything we do should have a purpose. Then what is the purpose of code review? The development period is already very tight, especially for Internet companies in China, where the boss wants you to work around the clock. Why do you need to spend so much time on code review? I think the purpose of code review is to improve code quality.

A few days ago, I read an article, in which there was such a paragraph that touched me a lot:

How did some of Facebook’s open source technology solutions come out of this model of tight business requirements, and were they developed by non-business teams? What I want to say is that even if the business needs are tight, I still write the code well. In addition, there are excellent Tech leads and strict code review, and the overall quality is not very bad. There is one bad thing in China: there is often no code review; And technical people have a bad concept, to write code as a job, as long as it can be done and used. So it gets more and more fucked up. Code Reivew has been a quality control tool for silicon Valley’s leading Internet companies, from Apple to Google, from Facebook to Airbnb and Uber. Sadly, people in China are too smart to think it is trivial and slow down development. Sometimes, we’re just too smart.

So let’s not always use lack of time as an excuse, more time doesn’t matter if you don’t have a commitment to code quality. Business constraints need to be addressed by increasing productivity, not by eliminating effort to improve code quality. Also, over the life of a project, does bad code really take less time to write than good code?

What are the most important characteristics of good code?

Since the purpose of a code review is to improve code quality, what makes good code? The first title I wrote was “What makes Good code?” “Then I realized that the problem was too big to give a simple definition of” good code “and would probably require a separate article, so I stopped talking about it and replaced it with a simple” What are the most important characteristics of good code?” .

I think the most important characteristic of good code is readability, so that your fellow students and future self can understand it without thinking too much, after all, the time spent maintaining the code far outstrips the time spent writing it. Each new line of code adds another maintenance cost, and readable code minimizes maintenance costs.

So how do we define readability? Everyone has their own standards. How do you get everyone to agree with you in a team? An engineer on Weibo wrote in his post about Bad Code:

Many books on code quality emphasize the idea that programs should be seen first and executed by machines second, and I tend to agree with that view. When evaluating whether a piece of code can be understood, I used to ask the author to translate the code word for word into Chinese, try to form a sentence, and then read the Chinese sentence to another person who has not read the code. If the other person can understand the code, the readability of the code is basically qualified. The reason for this is simple: this is what other people do when they understand a piece of code. The person reading the code will read the sentence word by word and infer the meaning of the sentence. If the sentence doesn’t make sense, they will need to understand the code in context. If the context doesn’t make sense, they may need to know more details about other parts of the code to help them infer the meaning. In most cases, the more context you need to understand what a sentence of code is doing, the worse the quality of the code. The benefit of word-for-word translation is that it makes it easy for the author to spot assumptions and readability traps that are not reflected in the code but are only known to him. Most code that doesn’t translate literally is bad code, like “ms for messageService,” or “Ms. Proc () for sending a message,” or “TMP for current file.”

I agree with this statement, and on this basis, I have always maintained that while a person who can describe something clearly may not make it readable, a person who can’t clearly describe something will make it very readable.

Three, then, how to do?

Said so much, specific how to fall into the real work? Even if you translate the code word for word into Chinese as mentioned above and explain it to other students, they may not understand it due to cognitive problems. For example, others may not understand the basic concepts that you think are very basic. So I think there are some basic principles that you need to follow in order to communicate effectively.

These are the five basic principles of object orientation, which I will not expand into here

  • Single responsibility principle

  • Open/closed principle

  • Liskov substitution principle

  • Interface segregation principle

  • Dependency inversion principle

The general understanding of this principle is not to copy the original code for the same functionality, but to abstract out a common method. But actually it is a waste to implement the same function with different ideas or code. Such as common log handling, exception handling logic.

3.3 Prefer Composition to Inheritance this principle has some overlap with Interface segregation principle in OOP’s SOLID principle mentioned above. With the continuous iteration of business requirements, small components will gradually evolve into large components, which will gradually become more difficult to control. If small components are continuously abstracted in the process of continuous iteration, it can keep the code concise while the business function is complex. For example, whether an airplane or a car or a train can move, I only need to know that the object is mobile when I use it. I don’t care whether the object is an airplane or a car.

3.4 Coding specifications Without further ado, you can use some of the best coding specifications in the industry. However, it is important to note that the purpose of the specification is to maintain the unity of the project coding style, and do not make meaningless arguments on the specification.

3.5 If you do not have the ability to abstract, then repeat it. This is a cruel and common reality. After reading a lot of books and wasting the efforts of the boss, you finally abstract a component, but the final result is to increase the maintenance cost. So if you feel like you can’t abstract it well, just use the crudest code you can, because it’s still readable, rather than having to write a bunch of ifs and else after abstraction.

Tips beyond technology

There are a few other points to pay attention to outside of the technology, the first and most important thing is to have an open mind, review the code, not the specific person, do not feel ashamed of the review, certainly do not make personal attacks.

Secondly, we should grasp the granularity of review, and do not launch a very big PR at once, which will put the review students under great pressure. For example, it is best not to have both refactoring and new feature development in a PR, or to submit a PR together until the last version is complete. Review should be an ongoing process, not something like a summary of milestones.

Thirdly, code review should not assume the responsibility of finding business logic errors, which are commonly referred to as bugs. Bugs should be guaranteed by unit testing, functional testing, performance testing and other methods, and code review should not be given too much responsibility.