What is Code Review?

Code Review, also known as Code Review, is a deliberate and systematic effort to call in other programmers to examine each other’s Code for errors.

Usually Code Review will have the following effects:

  • Improve code quality and maintainability, readability, etc.
  • Find some potential problems and so on.
  • Best practices, ways to get things done better and faster.
  • Knowledge sharing, Review others code, in fact, is also a learning process, my own reflection and summary.

Why Code Review?

Whether Code Review is needed needs to be decided according to the current environment and situation. If the development task of the project team is extremely tight, Code Review at this time may receive adverse effects. The core of Code Review lies in seeking common ground while reserving differences.

Code Review in particular requires communication and cooperation with project team members, and also checks the technical level of each member.

In particular, it is necessary to pay attention to the inertia of people, everyone has their own set of norms and norms, if you want to unify their behavior or norms, it is easy for them to produce resistance.

Before the Code Review, I communicated well with the project members to have a common vision, and assisted with corresponding training to make up for the weaknesses of some of them. It will relieve some of the discomfort of project members and make the project run more smoothly.

At the same time, it is also important to be careful not to be perfectionist in everything and take one step at a time. You know, slow is fast.

Prerequisites for Code Review

Code Review itself is an “after the fact” work, which can play a role in identifying the omissions.

When implementing Code Review, the following work should be prepared in advance so that the team can accept and implement it faster and better.

  • Establish specifications that include:
    • Code specification, such as variable naming, file naming convention, etc
    • Design principles, such as single responsibility principle, least knowledge principle, etc
    • Branch Management Policy
  • Perfect documentation for easy access. It is best to build the content of the document, do not appear “one word hall”
  • Develop Code Review process & goals and implementation cycle.

For example, in my current project team, the implementation of Code Review is divided into three stages according to the degree of difficulty.

  • The first phase focuses on proper annotation of functions, “hard coding” problems, common variable naming rules, etc., with an expected implementation period of 1 to 3 months
  • The second phase focuses on code coupling, single responsibility, minimum knowledge principles, potential hazards, performance issues, etc. The expected implementation period is 3-6 months
  • The third phase focuses on module implementation solutions, design patterns, best practices, code refactoring, etc.

In the process of implementation, if there is great resistance or difficulty, and the revenue from the implementation of Code Review is low, consider taking a “rest” period.

How is Code Review implemented?

Based on the experience of the implementation process of the current project and the experience of others, I suggest the following in Code Review:

  • You don’t need to review more than 500 lines of code at a time. You don’t have enough energy to review too much code at once.
  • A single review is recommended to last no longer than 30 minutes

Common Code Review item

Git Commit Specification

Git Commit specification:

  • Do not label information
  • Don’t commit in time
  • The information annotated when submitting is not standard

In addition to the general description information, you can also note by type, etc. :

  • Feat: new features
  • Fix: Fix the problem
  • Refactor: Code refactoring
  • Docs: Modify the document
  • Chore: Other modifications
  • Test: Test case modification
  • Style: code format modification, etc

Of course, we also need to use some tools to help us complete basic constraints and checks. For example: Using Git gracefully

style

1. Readability

There are good practices for measuring readability, namely whether the Code logic can be easily understood in Code Review. If not, it means that the readability of the Code needs to be improved.

Named after 2.

  • Naming is very important for readability. My personal preference is that the function/class name is not too long, but it must clearly indicate what the function/class does.
  • It is worthwhile to use English words as accurately as possible, even if you need to use translation tools. One caveat, though, is that if you’re using a word that’s unusual and no one knows it, don’t use it.

3. Function body length/class length

  • The body of the function is too long to read. It is generally recommended that no more than 50 lines be used
  • If the class is too long, say more than 10,000 lines, you might want to look at whether the “single responsibility” principle is violated.

4. Number of parameters

Not too many, generally not more than five, more than five, it is recommended to use objects

5. Comments

Proper comments can help us understand what functions/classes do.

Architecture/Design

1. Single responsibility principle

This is the most frequently violated principle, and the most difficult one to apply well.

  • A class does only one related thing
  • A function/method, preferably one thing

2. Whether the behavior is uniform

1) What is behavioral unity? Such as:
  • Whether error handling is uniform
  • Whether the error messages are consistent
  • The dialog box is displayed
  • .
2) The same logic/behavior has not executed the same code path

One of the characteristics of low-quality code is that the same logic/behavior is triggered in different places or in different ways, without executing the same code path (producing different results), or having a copy of the implementation everywhere, making it very difficult to maintain.

Code pollution

Is the code strongly coupled to other modules

4. Repeat code

It mainly depends on whether there are public components, reusable code, functions extracted

5. The open-closed principle

The simple way to think about it is to see how well the code expands

6. Robustness

  • Is core data forcibly checked?
  • Whether the business is considered complete, whether the logic is robust
  • Are there any potential bugs?
  • Is there a memory leak? Are there cyclic dependencies?

7. Error handling

Is there good Error Handling? Such as network error, IO error and so on.

8. Interface oriented programming/Object-oriented interface programming

It depends on whether the proper abstraction is done to abstract some behavior into an interface

9. Efficiency/performance

  • Whether the client program handles time-consuming operations such as frequent messages and large data properly
  • What is the time complexity of the key algorithm? Are there potential performance bottlenecks?

10. Code refactoring

  • Is the new change a patch that worsens code quality, or does it help improve code quality?

Common characteristics of low-quality code

  • Violates the “single duty” principle
  • The same logic/behavior, fired in a different way, does not execute the same code path
  • The lack of annotation
  • Poorly named functions/classes, poorly used words, or “selling dog meat “, for example
export function setDefaultImg(res) {
    let imgUrl = getImagePrefixUrl()
    let previewImgSrc = require('@/asserts/images/icon.png')
    if(res.imgId) {
        previewImgSrc = `${imgUrl}${res.imgId}.${res.type}? t=The ${+new Date()}`
    }
    return previewImgSrc
}
Copy the code

If the value is invalid, take the default image, otherwise return the img SRC relative address. From the perspective of function implementation, there are several problems as follows:

  • The setDefaultImg function name is not appropriate, there is no update/modification to the data, the ultimate goal is to get the img splice address (or the default image).
  • parameterresIs an object with only two basic types of properties actually used. Follow the least knowledge principle and modify the parameters.
  • let imgUrl, is actually the prefix of the spliced URL, named asimgPrefixUrlWould be more appropriate.
  • Let is improperly used, for examplelet imgUrlBecause there is no variable assignment involved, useConst is more appropriate
  • res.imgIdThat is, the handling of abnormal cases, less rightres.typeThe processing of
  • Variable names such aspreviewImgSrcThe purpose for which the SRC is used is uncertain. A name with a more general meaning is recommended.

It can be considered to be reconstructed as:

export function getImgSrc(imgId, imgType) {
    if(typeofimgId ! = ='string' || typeofimgType ! = ='string') {
        return require('@/asserts/images/icon.png')}return `${getImagePrefixUrl()}${imgId}.${imgType}? t=The ${+new Date()}`
}
Copy the code

A link to the

  • JavaScript design Principles