Code review can be a real pain for a team that begins implementation. You are sure to fall into many traps: review may take more time than coding; location of the brackets can cause deadly holywars; and disputes on whether or not the branch can be merged into the master before the team approves it could last forever. I’ve put together a number of techniques that will help you smoothen the adaptation process quite a bit — at least, they did turn out to be helpful to me.

This material is a brief summary of my experience gained through several years of my work in large mobile development teams. For the most part, it relates to mobile development, which had some influence on my examples and references.

Code Review Goals

To begin with, I will outline briefly the goals of the code review. It is these goals and their relative priority that I check before recommending a certain technique.

1. Collective ownership of the code

Decreasing the bus factor by ensuring that each component of the code has been seen and analysed by more than one pair of eyes, and the knowledge does not belong to one specific developer only.

2. Code reuse

When several people who are responsible for various work tasks and possess different knowledge about the product look at each other’s code, they can spot certain patterns or suggest reusing one of the pre-existing solutions, or to put the current solution into a separate library.

3. Knowledge sharing in the team

Each team member has some knowledge they can share. Someone is really good at algorithms; someone knows a lot about metaprogramming; and there is also someone who is a real MVC Jedi. Having a chance to look at each other’s solutions and ask questions offers a great opportunity to raise the overall technical level of the team as a whole.

4. Error diagnosis

The most obvious goal is to detect errors of different severity. This is the first thing your manager thinks about when you suggest that they allocate a certain period of time for code review.

5. Project consistency

Every programmer has their own opinion about how to do coding. Tabs and blanks, file structure of the project — lack of consistency can make code reading and analysis a lot more difficult.

Let’s get to business now.

Code Review Models for Different Teams

Teams can be very different. One company can have thirty people in one department working on one and the same project, while in another, one developer single-handedly supports dozens of applications.

Small Team

The first case is probably the most common one. It is a team with a few members, let’s say, up to four, who all work on the same project. This is where we normally see Gif Flow thriving; the tasks get developed independently in separate branches and then get merged in Develop when complete.

The best fit for this case would be a model where one needs the approval of all team members before merging the branch with the task to the main work branch. This gives maximum benefits for all code review goals mentioned in Section 1.

Several Independent Teams

Let’s scale up the previous situation and imagine a large outsourcing company having several small teams similar to the one described above, and each of them is working on their own small project. In cases like this, the hierarchical structure usually becomes more complicated. In addition to the lead developers, each team also has a team lead or a tech lead who is responsible for several such projects at once.

The basic approach remains unchanged — reviews are conducted for all new tasks and require the entire project team, but the final responsibility for all technical decisions rests with the lead developer. Besides, important reviews must also involve the lead. This way the lead will know what’s going on with the project and step in when his/her help is needed to solve a problem.

Another activity can be quite useful. Periodic cross-project reviews when the developer learns about the other projects proved highly efficient. These reviews help everyone be generally aware of the technologies and approaches used, and the areas of expertise of the project participants. There may be different approaches to cross-project reviews. It could be a list of all projects sorted by time, or every developer can inspect other projects periodically. Such reviews are conducted mainly for information purposes, and the approval of an external reviewer is not required to close the task. The person reviewing the code learns from other teams.

Large Team

Another case is a large team with all the members working on one large-scale project. It is reasonable to set a certain limit to the number of reviewers approving a branch to be merged. This number can depend on the size of the team, but 2 to 4 people are usually enough.

The big question is how to choose reviewers — they can be selected randomly (whoever responds first) or designated by the review author. This process can be automated in order to avoid disputes and ensure there is no bias. For example, the code owner can be identified based on the history of the revision control system or of the code review system.

Single Developer

The last case is also the bitterest one. The developer is working alone in the team. First of all, the goal of collective code ownership is no longer applicable — no group, no ownership. The rest of the goals also become rather senseless. However, an external look at the code will help find some errors and increase the technical level in general.

In this case, the developer needs to turn to the open community. I can recommend you three channels.

1. Various chats and communities of developers

My personal russian favorites are Cocoa Developers Club Slack Chat and iOS Good Talks Telegram Chat. You may also look at iOS Developers HQ for English speaking developers.

2. Meetings of developers

For example, Peer Lab that is held all over the world every week.

3. Joint work with a colleague from another function

You can involve a colleague who works on development for a different platform — Android, Web or Backend. But be careful — not everyone is prepared or willing to focus on issues from a different development field.

Code Review Practices

Scheduled Operation

The main issue here is the flow state. Pulling a programmer out of the flow state can be quite destructive. Just imagine: you are neck-deep in the development of your feature, building castles of abstractions in your mind, when suddenly your colleague gives you a shy tap on the shoulder asking to take a look at his code. Nothing good can come out of a situation like this — the developer can no longer do their work normally and is too biased and annoyed to review the work of others.

A good solution here is a schedule. Each developer in the team decides the time of the day when they are able to review. The schedule of the whole team can be kept in one table, so that one could use it as a guidance when choosing reviewers for themselves or calculating the time required for review of the task. For example, I have always allocated for it an hour of my time in the morning, right when I come in for work — this helped me get into the swing of things.

Of course, there are cases when this mechanism fails — some reviews really need to be done as quickly as possible. The most important thing is not to make a habit out of situations like this, but to stick to the routine order.

Architecture Reviews

I am sure you came across some situations like this — after the task has been implemented, it turns out it had to be done in an absolutely different way. Code review is not the best place to argue about the global architectural decisions. The code is already finished, the task is almost done, and recoding from zero will be too costly. Such things cannot be postponed; they need to be addressed beforehand.

A good option is to conduct an architecture review and defend your component before the team in words or using a diagram. Of course, there are cases when a brilliant idea comes to someone as late as at the point of review. If the suggestion is really valuable, document it in the issue tracker and never return to it.

You can invite the project tech lead and developers who share an imminent interest in the implementation of the task, or simply any team member who wants to participate.

Review Scope Limitation

There is an interesting paradox — the less code is provided for review, the more comments the reviewers leave. This rule remains true for virtually any team. Just a couple of lines of code can provoke broad discussions and heated debates. But as soon as you increase the scope of changes up to several thousands of lines, the number of comments drops to one or two and remains unchanged.

This practice is very simple — don’t submit too many changes for review at once. This requirement correlates with the standard rules for the breakdown of tasks — each task should take no longer than one work day to complete. If too much code is submitted at once, the reviewers will lose focus quite easily, and many issues will simply be overlooked. Another requirement would also fit in here nicely — take away all the information noise that distracts the reviewers from the real meaning of changes.

Self Review

The authors can spot the majority of code errors themselves. That’s why the developer should look through the code themselves once or twice before asking colleagues to review their work. This simple procedure will help you save a lot of time and nerve and improve the relationships with your co-workers.

Review Description

I have already mentioned that when you conduct code review on a permanent basis, it becomes difficult to focus on the actual meaning of changes. One way to help reviewers maintain their focus is to provide a detailed description of the meaning of changes. I usually use the following template:

• Review name
• Brief description of changes
• Things that require special attention
• Linked tasks and materials

Check Lists and Their Automation

Check lists are cool. Use them for code review as well. Here are some examples of questions from such a check list: “Is there a code that has been commented-out?”, “Was the business logic covered by any tests?” “Were all the errors processed?”, “Are all the constants in one place?” Try to memorize the questions that the team members ask during the review and put them on the list.

Do not let this list grow too long. Think about the ways to automate each checkpoint on your list — given enough efforts, you are most likely to cover the majority of the needs. A great number of tools that can help with this task — linters, static analysis tools, and duplicated code detection tools.

Measuring Code Review Efficiency

Implementation of every process should go hand in hand with collection of metrics describing its efficiency and with analysis of such metrics. The process is not an end in itself. Code review is no exception. It is important to understand to what extent it helps the team to do their work.

A good option is to look at the goals of the code review. You can use the ones I listed in the first section as a template. One way to measure efficiency is to use the Goal-Question-Metric approach. It includes three stages — identify the goal you want to measure; list the questions that will help you understand the extent to which the goal has been achieved; and determine the metrics that help find answers to the questions. Let’s consider an example.

First, we identify the goal. Say we want to “Increase code reuse.” You can use several goals at a time and measure them independently from each other.

Now, let’s develop questions that will help us understand whether or not we are on the way to achieve the goal. In our case, the following questions seem reasonable: “Are we using shared libraries in the project?”, “Did we put the visual styles into a separate component?”, “Is there a lot of duplicated code?”

The last thing — let’s determine the metrics that will assist us in answering the questions. In our case, it will be the number of shared libraries, the number of locations with hard-coded styles, and the share of duplicated code.

It is quite easy to use this framework. We check the necessary metrics before implementing the process, determine the desired outcome, and once in a while, on a monthly or quarterly basis, repeat all the measurements again. If this can be automated, then it’s even cooler. After some time we analyse the outcome and compare the changes in metrics to judge the efficiency of the process.

But why is that metric really good? Imagine that instead of working on suggestions on code reuse developers had spent most of their time engaged in holy wars about blanks and tabs — with such reviews, the key indicators would have been unlikely to increase a lot; the desired outcome would not have been achieved; and we would not be able to understand that there was something wrong with the process.

Besides, it’s useful to collect some other metrics that can tell us what problems we faced using the process.

• Inspection rate— tells us how quickly the review was done,
• Defect rate— describes the number of defects detected per hour of review
• Defect density— refers to the number of defects found per code line.

If we collect these data for each review, we will be able to adequately assess the level of process engagement for each team member. A number of code review systems — for instance Crucible from Atlassian — automatically collect these data.

Conclusion

As I have mentioned in the introduction, this article is a brief summary of my presentation where I also talk about various process automation tools, team regulations and rules of conduct, and some other code review aspects.

Useful Links

• Examples of decent reviews: Wire, Artsy, Kickstarter.
• iOS Developers HQ.
• Peer Lab Official Website and Peer Lab Moscow Telegram Chat.
• About Goal-Question-Metric