April 22, 2012 at 19:03

Another article on code review

    What is code review


    Code review - engineering practice in terms of a flexible development methodology. This is an 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:
    • Improving code quality
    • There are "stupid" errors (typos) in the implementation
    • Increased code sharing
    • The code is reduced to a single style of writing.
    • It is well suited for training "beginners", skill is quickly gained, experience is equalized, knowledge is exchanged.


    What can be inspected


    Any code is suitable for a review. However, a review  must be carried out for critical places in the application (for example: authentication mechanisms, authorization, transfer and processing of important information - processing of money 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 an incorrect test can be very expensive.

    How to conduct a review


    In general, code reviews should be carried out in conjunction with other flexible engineering practices: pair programming, TDD, CI. In this case, maximum review efficiency is achieved. If a flexible development methodology is used, then the code review stage can be entered into the Definition of Done features.

    What review consists of


    • First, a design review is an analysis of the future design (architecture). This stage is very important, because without it the code review will be less useful or generally useless (if the programmer wrote the code, but this code is completely incorrect - it does not solve the problem, does not satisfy the memory requirements , 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, one does not find fault (writing style, error checking), but this algorithm is completely unsuitable for working time. 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, the code review itself is an analysis of the 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 the one who will implement the code (as in scrum during the planning poker), or to the 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 a table, in a circle of colleagues, at a marker board, in the corporate wiki. At the design review , the one who will write the code will talk about the chosen strategy (approximate algorithm, required tools, libraries) for solving the task. The whole charm of this stage is that the design error will cost 1-2 hours of time (and will be fixed immediately at review).

    How to conduct code review


    You can conduct a code review in different ways - remotely, when each developer sits at his workplace, and together - sitting in front of the 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 you to contain only verified code in the repository. Microsoft uses this approach: patches with changes are sent to all review participants. After the feedback has been collected and processed, the process is repeated until all reviewers agree with the changes.

    Post-commit review


    This type of review is carried out after making changes to the VCS. At the same time, you can commit both to the main branch and to the temporary branch (and pour already verified changes into the main branch).

    Thematic review


    You can also conduct thematic code review - they can be used as a transitional stage on the way to a full code review. They can be carried out for a critical piece of code, or when searching for errors. The most important thing is to determine the purpose of this review , while the goal should be clear and clear:

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

    The main difference between a thematic review and a full-fledged code review is their narrow specialization. If in code review we look at the code style, compliance with the implementation and statement of the problem, search for dangerous code, then in the thematic review we usually look at only one aspect (most often - analysis of the algorithm for compliance with TK, 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 of the brainstorming. We used this approach when searching for logical errors in our software: we looked at the "old" code that was written a few months before review (this can also be attributed to differences from the usual review - where they usually look at fresh code).

    Review results


    The most important thing when conducting a review is the use of the result. The following artifacts may appear as a result of review:
    • Description of the method for solving the problem (design review)
    • UML diagrams (design review)
    • Code Style Comments
    • A more correct version (quick, easy to read) of implementation (design review, code review)
    • Indication of errors in the code (forgotten condition in switch, etc.) (code review)
    • Unit tests (design review, code review)

    At the same time, it is very important that all the results are not lost, and that they are entered in the VCS, wiki. This may be prevented by:
    • Project Dates.
    • Laziness, forgetfulness of developers
    • Lack of a convenient review review mechanism, as well as control over these changes.

    To overcome these problems, it can partially help:
    • pre-commit hook in VCS
    • Creating a branch in VCS from which changes are poured into the main branch only after review
    • Disable distribution builds on a CI server without review. For example, when building the distribution kit, check for special properties (svn: properties), or a special file with the review results. And refuse to build the distribution, if not all reviewers have approved the code.
    • Use of methodology in development (in which code review is an integral part).



    Difficulties in conducting a review (subjective opinion)


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


    Utilities for review


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

    References






    Wishes, additions, criticism are welcome
    Tags:

    Also popular now: