Another article on code review

What is code review

Code review - engineering practice in terms of flexible development methodology. This analysis (inspection) of the code in order to identify errors, shortcomings, discrepancies in the style of writing code, in accordance with the written code and the task.

The obvious advantages of this practice include:

• Improved code quality
• There are "stupid" errors (typos) in the implementation
• Increased code sharing
• The code is reduced to a single style of writing.
• Well suited for training "beginners", quickly gaining skill, leveling experience, knowledge sharing.

What can be inspected

Any code is suitable for review. However, the review must necessarily be held for critical places in the application (for example: authentication mechanisms, authorization, transfer and processing of important information - processing cash transactions, etc.).
Unit tests are also suitable for review, since unit tests are the same code that is prone to errors, it must be inspected as carefully as the rest of the code, because a wrong test can be very expensive.

How to conduct a review

In general, code review should be carried out in conjunction with other flexible engineering practices: pair programming, TDD, CI. In this case, the maximum performance review is achieved. If a flexible development methodology is used, then the code review step can be added to the Definition of Done features.
')

What does review consist of

• First, the design review is an analysis of the future design (architecture). This stage is very important, because without it, reviewing the code will be less useful or even useless (if the programmer wrote the code, but this code is completely incorrect, it does not solve the task, does not meet the requirements for memory time). Example: the programmer was tasked with writing an array sorting algorithm. The programmer implemented the bogo-sort algorithm, and from the point of view of the quality of the code, it is impossible to find fault (writing style, checking for errors), but this algorithm does not fit the work time at all. Therefore, the review in this case is useless (of course - this is an exaggerated example, but I think the essence is clear), here it is necessary to completely rewrite the algorithm .
• Actually, code review itself - analysis of written code. At this stage, comments are sent to the author of the code, wishes on the written code.

It is also very important to decide who will have the last word in making the final decision in the event of a dispute. Usually, priority is given to those who will implement the code (as in scrum when conducting a planning poker), or to a special person who is responsible for this code (as in google - code owner).

How to conduct a design review

Design review can be carried out at the table, in a circle of colleagues, at a marker board, in a corporate wiki. On design review , the person who will write the code will tell about the chosen strategy (approximate algorithm, required tools, libraries) of the solution of the problem. The beauty of this stage is that the design error will cost 1-2 hours of time (and will be eliminated immediately for review).

How to conduct code review

You can conduct code review in different ways - remotely, when each developer is sitting at his workplace, and together - sitting in front of a monitor of one of his colleagues, or in a specially designated place, for example, a meeting room. In principle, there are many ways (you can even print the source code and make changes on paper).

Pre-commit review

This type of review is carried out before making changes to the VCS. This approach allows keeping only verified code in the repository. In Microsoft, this approach is used: patches are sent to all participants with changes. After the feedback has been collected and processed, the process is repeated until all the reviewers agree to the changes.

Post-commit review

This type of review is carried out after making changes to the VCS. In this case, it is possible to commit both to the main branch and to the temporary branch (and to add already verified changes to the main branch).

Thematic review

You can also carry out thematic code review - they can be used as a transitional step towards a full-fledged code review. They can be carried out for a critical section of code, or when searching for errors. The most important thing is to determine the purpose of this review , and the goal should be foreseeable and clear:

• " Let's look for errors in this module " - is not suitable as a goal, since it is boundless.
• " Analyzing the algorithm for compliance with RFC 1149 specifications " is already better.

The main difference between thematic review and full-featured code review is their narrow specialization. If in code review we look at the style of the code, the conformity of the implementation and formulation of the problem, the search for dangerous code, then in the thematic review we usually look at only one aspect (most often the analysis of the algorithm for compliance with the TOR, error handling).
The advantage of this approach is that the team gradually gets used to the practice of review (it can be used irregularly, on demand). It turns out a kind of analogue brainstorming. We used this approach when searching for logical errors in our software: we looked at the “old” code that was written several months before the review (this can also be attributed to differences from the usual review — where the latest code is usually viewed).

Results review

The most important thing when reviewing is to use the result obtained. As a result of the review, the following artifacts may appear:

• Description of how to solve the problem (design review)
• UML diagrams (design review)
• Comments on the code style (code review)
• More correct option (fast, easy to read) implementation (design review, code review)
• Indication of errors in the code (forgotten condition in the switch, etc.) (code review)
• Unit tests (design review, code review)

It is very important that all the results are not lost, and were included in the VCS, wiki. This can be hindered by:

• The timing of the project.
• Laziness, forgetfulness of developers
• The lack of a convenient mechanism for making changes review, as well as control over the introduction of these changes.

To overcome these problems, it may help in part:

• pre-commit hook in vcs
• Creating a branch in VCS, from which changes are merged into the main branch only after review
• Disable build distributions on the CI server without reviewing. For example, when building a distribution kit, check special properties (svn: properties), or a special file with the results of review. And refuse to build the distributive, if not all reviewers approved (approve) code.
• Using methodology in development (in which code review is an integral part).

Difficulties in reviewing (subjective opinion)

The main difficulty we encountered when we introduced review for the first time: it is the inability to control changes based on the results of the review. This is partly due to the fact that this practice was applied without other practices - CI (this again proves the fact that all engineering practices should be applied together).

Utilities for review

In general, there are a large number of utilities for review, both paid and free. I did not give them so as not to impose my point of view; on the Internet one can find a lot of tools and a detailed description.

Links

• http://blog.not-a-kernel-guy.com/2007/02/21/151
• http://www.rsdn.ru/forum/management/3350124.flat.aspx
• http://ostatic.com/blog/open-source-code-review-tools
• http: //xn-----clcksaplxf6byd3cyb.xn--p1ai/

Wishes, additions, criticism is welcome