5 Major Problems With Synchronous Code Reviews

Herbert Bodner, https://www.fraankly.com/, @Herbert_Bodner

If your teams does not use git as a source control system then you should continue reading. Because then you most likely use the synchronous code review type and that's giving you 5 major problems.

But even if you do use git as a source control system then you still might use the synchronous code review type. If so, then you still have these 5 problems - even though you use git.

With synchronous review I mean a code review, that is performed together in front of the coder's screen immediately after the coder finished coding.

In contrast, the asynchronous review is done by the reviewer on his own screen, on his own schedule. The reviewer uses some tools to write comments, that are then forwarded to the coder to improve the code.

If you want to know more details about the differences of code reviews I recently wrote a separate blog post about the 4 types of code reviews.

At the time of this writing my team uses Team Foundation Server (TFS) and its built-in source control TFSC (Team Foundation Source Control). Most of the time we also use synchronous code reviews and I found a couple of problems with that approach.

Ok, so what are the downsides of synchronous code reviews?

5 major problems with synchronous code reviews

Synchronous code reviews produce following problems:

1. Direct dependencies
2. Forces context-switching
3. Not done consistently
4. Time pressure
5. Lack of focus

Let's have a look at each of these issues in more detail.

1) Direct dependencies

A big problem for the synchronous code review is the fact that the reviewer has to review based on the coders schedule. Immediately when the coder is finished, the reviewer is expected to stop with his task and start the review together with the coder.

In such a situation the coder depends on the reviewer and has to wait until the reviewer is available. And it is not worth for the coder to start with a new task, because the reviewer should be available soon.

There is a direct dependency between the reviewer and the coder.

And direct dependencies are not good because the more direct dependencies you have in your process flow the higher the chance of blockers.

When the coder is requesting a review the reviewer might be busy with something else, maybe with something more important.

He might tell the coder that he will be available for the review soon.

And the coder is waiting, and waiting, and waiting.

5 minutes, maybe 10 minutes later they finally start with the review. During that time the coder was just waiting doing nothing, except maybe checking his facebook news feed.

In contrast, when using asynchronous code reviews the coder does not depend on the reviewer. When the coder is finished he will use some tools to create a review request and continue with his next task.

With asynchronous code reviews there is no direct dependency between the coder and reviewer and therefore no potential blocker.

2) Forced context-switching

Imagine you are working on your own task and are in the middle of implementing a complex algorithm.

Then your teammate is asking for a review because he just finished his task.

This means that you have to stop working on your own task and review the task of your coworker.

Because of this you are dragged out of the context of your own work.

This has some negative side effects: context-switching is exhausting and frustrating. And it will take you a few moments to focus on the new context.

I have been in the situation as a reviewer by myself a couple of times and I don't like it.

When the coder started to explain his code to me I had to ask him to repeat the last one or two sentences once more. My brain was still processing the work I had been doing just before and therefore I hadn't been able to listen to the new context.

After the review is finished you have to switch the context again. It will take you again some time to come back to your own task and the context, where you left off.

In contrast, when using asynchronous code reviews the reviewer can review the code when it fits his own schedule. He is not forced to switch the context based on the the coders schedule

The reviewer can finish with his task first and only then start with the review, reducing the amount of required context-switches to a minimum.

3) Not done consistently

Imagine you are starting to work on a new task.

At the beginning you start to investigate the existing code base to figure out which of the existing methods and classes can be reused.

Let's assume that a lot of the required functionality is already there and you only have to put the pieces together by calling a couple of methods.

In the end there are just a few lines of code that you had to change to achieve the goal of your task.

Then you wonder whether it is really necessary to ask your colleague for a review. The code changes are actually so simple that it is almost impossible that a review might lead to any improvements.

Therefore, you decide to skip the review and just go ahead and commit the changes. You save yourself and also your colleague some time.

This is a natural and understandable behaviour, but it is dangerous.

How do you decide when a code change is simple enough so it does not require a review?

It is very hard to define rules for that.

Review everything

So the best rule is to just review everything, no matter how small and simple the code changes are.

But even then you most likely have a problem that people are not going to follow this "review everything" rule.

It is just additional effort to get a reviewer to your desk for such a small code change. You have to ask your colleague whether he has time for a review, wait for him to roll over, explain what you were doing, etc.

The barrier for performing a review is actually quite high when using a synchronous code review approach.

In contrast, when the team uses an asynchronous code review approach, then it is quite easy to create a pull request and assign it to your colleague. So requesting a code review is way simpler when you use an asynchronous review approach.

Therefore, the likelihood that a code review actually happens is also higher when using an asynchronous code review compared to a synchronous one.

This argument is implicitly also supported by a survey conducted with 550 developers in 2017.

One finding of this survey was that developers who use a code review tool are four times more likely to review on a daily basis than those using a meeting-based approach.

This result is definitely not a perfect proof for the claim that asynchronous reviews happen more often compared to synchronous ones. But I think you can safely assume that you need additional review tools for an asynchronous review type to give feedback to the code. In contrast, for a synchronous review you don't need that much tooling, although it might help.

Therefore, the likelihood that code reviews are done consistently is higher for the asynchronous approach compared to the synchronous one.

4) Time pressure

Let's assume your team is using synchronous reviews and your coworker just finished his task and asked you to perform a review.

So you join him at his desk and he starts to explain about the task and the complex problems he had to solve as part of his task.

As he was spending the last couple of hours to analyze that problem and to find a solution he has a lot of insights and knowledge about that task. Therefore the explanations are very sharp and right to the point.

But it is quite difficult for you as a reviewer to follow. You didn't invest the last couple of hours to analyze the problem. You didn't put that much thought into finding a good solution.

Therefore it is very hard for you to decide within a few moments whether the presented solution is the best or not.

But your colleague wants to get his code approved as soon as possible.

This puts you, as the reviewer, in a situation of pressure. On the one hand you want to take the time to think the solution through because you want to validate, whether it is really a good solution. But on the other hand you want to approve the solution so your colleague can move on.

In this situation you are not encouraged to question the solution too much. Even if you don't understand exactly how the new code works and why it has been implemented that way, you are not going to ask too many questions.

Why? There are three reasons for this:

Trust in coder

First of all your coworker might explain his solution in a very self-confident manner. This builds trust and you might think, that he knows what he is doing. He was working on this for the last couple of hours and the solution seems to be solid and sound.

However, trust is good in a team in general, but not for a code review.

Actually the whole point of a code review is to distrust your colleague and double check everything to make sure the code is really doing what it is supposed to do.

Avoidance to ask questions

Second of all, you as a reviewer don't want to ask too many questions, because it might look like that you are not able to follow the explanations - or you are just too slow to understand what is going on.

Therefore it is easier to not ask too many questions, because you don't want to look like a fool.

Avoidance to review same code lines multiple times

Third of all, you don't want to look through the same piece of code multiple times, when the coder is sitting next to you. Even if you review a complex algorithm that is hard to understand when looking at it for the first time, you are not going to switch back and forth between different code files for a couple of minutes while the coder is waiting next to you.

Because then it might appear that you are not able to grasp the solution. And your mind is just to slow to understand the problem and figure out the solution.

So what happens is that you are not going to look too close into validating the solution. You don't have the time to think everything through, because of the pressure of having the coder sitting next to you.

Naturally, this results in worse code quality because the review is performed under time pressure and therefore you might miss some potential issues in the code.

In contrast, with an asynchronous review you work on your own schedule and are not under time pressure. You can take as long as you need to understand and review the solution. Therefore it is more likely that you find the bugs before you approve the code.

5) Lack of focus

Let's say you are in the middle of coding - working on a new complex feature and you are totally focused to implement this new algorithm you have in your mind.

While you are highly concentrated your team mate next to you just finished his task and asks you for a review of his code.

Of course, you are not happy about this.

But you cannot decline the request because your colleague is waiting. He is blocked until you review his code. Only when the code is approved and committed he can start to pick up the next task.

As you are a nice person you will roll over with your chair to your teammate and do the review.

But you are frustrated.

You want to get back to your own work as soon as possible.

And this feeling has an impact on the quality of code review you perform. You want to get it done as fast as possible, so you can continue with your complex algorithm.

Therefore you don't look too close and just do a quick and dirty review without putting a lot of effort in it.

This has the effect that you might miss some defects in the code, which you normally would have spotted.

As a matter of fact nobody likes to get interrupted while working on a complex task that requires highest focus and attention.

However, when the team performs asynchronous code reviews you don't have these problems.

You perform the review when it fits your own schedule, when you have the time and the full mental capacity, then you can put all your attention to the code to review.

How to fix these problems?

Ok, so now I hope I convinced you that synchronous code reviews are bad, if they are used as the default code review type within a team.

There are of course some cases where the synchronous type works best, as I described here, but these cases should not be the norm.

As I mentioned at the beginning my team uses at the time of this writing the synchronous approach as the default review type as well. And we do face all the above mentioned problems.

What are we doing to solve these issues?

Well, we are in the process of moving our codebase to git. With git in place and with the tooling provided by TFS we will be able to use asynchronous code reviews.

I am really looking forward to that, but it will still take some time to prepare the move.

This article was originally published on https://www.scrum-tips.com/2018/05/22/synchronous-code-reviews/ and is reproduced here with permission from Herbert Bodner.

---

Related code reviews resources

Code Review: Why It Matters

Four ways to a Practical Code Review

Code Reviews for Test Automation Code

---

Click here to view the complete list of Methods & Tools articles

This article was published in August 2020