5 Major Problems With Synchronous Code Reviews

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.

Ok, that’s it for today. Let me know in the comments section if you have any remarks. Take care and HabbediEhre!

4 Types Of Code Reviews Any Professional Developer Should Know About

Every professional software developer knows that a code review should be part of any serious development process. But what most developers don´t know is that there are many different types of code reviews. And each type has specific benefits and downsides depending on your project and team structure.

In this post I am going to list the different types of code reviews and explain how each type works exactly. I am also going to give you some ideas on when to use which type.

Ok, let’s get started.

Here we go.

First of all, on a very high level you can classify code reviews in two different categoriesformal code reviews and lightweight code reviews.

Formal code review

Formal code reviews are based on a formal process. The most popular implementation here is the Fagan inspection.

There you have a very structured process of trying to find defects in code, but it is also used to find defects in specifications or designs.

The Fagan inspection consists of six steps (Planning, Overview, Preparation, Inspection Meeting, Rework and Follow-up). The basic idea is to define output requirements for each step upfront and when running through the process you inspect the output of each step and compare it to the desired outcome. Then you decide whether you move on to the next step or still have to do work in the current step.

Such structured approach is not used a lot.

Actually in my career I have never came across a team that used such a process and I don’t think I will ever be able to see that.

I think it is because of the big overhead that this process brings with it and therefore not a lot of team make use of it.

However, if you have to develop software that could cause the loss of life in case of a defect, then such a structured approach for finding defects makes sense.

For example if you develop software for nuclear power plants then you probably want to have a very formal approach to guarantee that there is no bug in the delivered code.

But as I said, most of us developers are working on software that is not life-threatening in case of a bug.

And therefore we use a more lightweight approach for code reviews instead of the formal approach.

So let’s have a look at the lightweight code reviews:

Lightweight code reviews

Lightweight code reviews are commonly used by development teams these days.

You can divide lightweight code reviews in following different sub categories:

  1. Instant code review—also known as pair programming
  2. Synchronous code review—also know as over-the-shoulder code review
  3. Asynchronous code review—also known as tool-assisted code review
  4. Code review once in a while—also known as meeting-based code review

Type 1: Instant code review

The first type is the instant code review, which happens during pair programming. While one developer is hitting the keyboard to produce the code the other developer is reviewing the code right on the spot, paying attention to potential issues and giving ideas for code improvement on the go.

Complex business problem

This type of code review works well when you have to solve a complex problem. By putting two heads together to go through the process of finding a solution you increase the chance to get it right.

Having two brains thinking about the problem and discussing possible scenarios it is more likely that you also cover the edge cases of the problem.

I like to use pair programming when working on a task which requires a lot of complex business logic. Then it is helpful to have two people think through all the different possibilities of the flow and make sure all are handled properly in the code.

In contrast to complex business logic, you sometimes also work on a task, that has a complex technical problem to solve. Here I mean for instance you make use of a new framework or explore a piece of technology you never used before.

In such a situation it is better to work by yourself because you can work on your own base. You have to do a lot of searching on the web or reading documentation on how the new technology works.

It is not helpful to do pair programming in a such a case, because you hinder each other while getting the required knowledge.

However, if you get stuck then talking to a colleague about the solution often helps you to view the problem from a different angle.

Same level of expertise

Another important aspect to consider when doing pair programming is the level of expertise of the two developers working together.

Preferably both developers should be on the same level because then they are able to work along in the same speed.

Pair programming with a junior and senior does not work very well. If the junior has the steering wheel then the senior next to him just gets bored because he feels everything is just too slow. In such a setting the potential of the senior gets restricted and therefore is a waste of his time.

If the senior has the keyboard in his hand then everything goes to fast for the junior. The junior is not able to follow the base of the senior and after a few minutes he loses the context.

Only if the senior slows down and makes sure he explains to the junior on a slower pace what he is about to do, then this setup makes sense.

However, then we are not talking about pair programming anymore. Then we are talking about a learning session, where the senior developer teaches the junior developer how to solve a specific problem.

But if both developers are on the same level then it is amazing how much work they can accomplish in such a setting. The big benefit here is also that the two developers motivate each other and in case of one of them loses focus the other developer brings him back on track again.

To sum it up: pair programming works well when two developers with a similar level of experience work together on solving a complex business problem.

Type 2: Synchronous code review

The second type is the synchronous code review. Here the coder produces the code herself and asks the reviewer for a review immediately when she is done with coding.

The reviewer joins the coder at her desk and they look at the same screen while reviewing, discussing and improving the code together.

Lack of knowledge of reviewer

This type of code review works well when the reviewer lacks knowledge about the goal of the task. This happens when the team does not have refinement sessions nor proper sprint planning sessions together, where they discuss each task upfront.

This usually results in the situation where only a specific developer knows about the requirements of a task.

In these situations it is very helpful for the reviewer to get an introduction about the goal of the task before the review is started.

Lots of code improvements expected

Synchronous code reviews also work well if there are a lot of code improvements expected due to the lack of experience from the coder.

If an experienced senior is going to review a piece of code that has been implemented by a very junior guy, then the review generally works way faster when they do the improvements together after the junior claims he is done.

Downside of forced context switching

But there is a major downside of synchronous code reviews, which is the fact of forced context switches. This is not only very frustrating for the reviewer, but slows down the whole team.

In fact, I have written a separate blog post about the 5 major problems with synchronous code reviews. Therefore I don´t get into more details about that type of code review here.

Type 3: Asynchronous code review

Then we have the third type, the asynchronous code review. This one is not done together at the same time on the same screen, but asynchronously. After the coder is finished with coding, she makes the code available for review and starts her next task.

When the reviewer has time, he will review the code by himself at his desk on his own schedule, without talking to the coder in person, but writing down comments using some tooling.

After the reviewer is done, the tooling will notify the coder about the comments and necessary rework. The coder is going to improve the code based on the comments, again on his own schedule.

The cycle starts all over by making the changes available for review again. The coder changes the code until there are no more comments for improvement. Finally, the changes are approved and committed to the master branch.

As you can see synchronous code reviews work quite differently compared to asynchronous ones.

No direct dependencies

The big benefit of asynchronous code reviews is that they happen asynchronously. The coder does not directly depend on the reviewer and both of them can do their part of the work on their own schedule.

Downside of many review cycles

The downside is that you might have many cycles of reviews, which might spread over a couple of days until the review finally is approved.

When the coder is done, it usually takes a couple of hours until the reviewer starts to review. Most of the time the suggestions made by the reviewer are then fixed by the coder only the next day.

So the first cycle already takes at least a day. If you have a couple of those cycles then the reviewing time spans over a week—and this is not even taking the time for coding and testing into account.

But there are options to prevent this long timespan to get out of hand. For instance, in my team we made the rule that every developer starts with pending reviews in the morning before he picks up any other task. And the same thing is done after lunch break.

As the developer is out of his context anyway after a longer break you don´t force unnatural context switching and still have the benefit of getting the code reviewed in a reasonable time.

Comparing the benefits and downsides of this type of code review I think that asynchronous code reviews should be the default type for every professional development team.

But before I tell you why I think that way, let’s have a look at the 4th type of code reviews.

Type 4: Code review once in a while

A long time ago I used to do code review sessions about once every month together with the whole team. We were sitting in a meeting room and one developer was showing and explaining a difficult piece of code he has recently been writing.

The other developers were trying to spot potential issues, commenting and giving suggestions on how to improve the code.

I don’t think that any teams use the once-in-a-while code review method on a permanent basis. I can only think of one situation when this type could makes sense: when the whole team has no experience at all with code reviews, then getting everyone together in a room and do the review together a couple of times might help everyone to understand the purpose and the goal of a code review.

However on a long-term the 4th type this is not an adequate technique, because it is rather inefficient having the whole team working through a piece of code.

Ok, now we have covered all types of code reviews.

So, now you might wonder which type you should choose.

Which code review type should I pick?

We talked about the formal type, which is obviously not so popular and hardly used in practice..

Then we talked about the category of lightweight code reviews and distinguished 4 different types.

Type 1, the instant code review, is done in pair programming and works well when two developers with a similar skill set are working on a complex business problem.

Type 2, the synchronous code review, works well when the reviewer lacks knowledge about the goal of the task and needs explanation by the coder. It also works well if there are a lot of code improvements expected due to the lack of experience from the coder.

But it has the downside of forced context switches, which is frustrating for the reviewer and slows down the whole team.

Type 3, the asynchronous code review, prevents the problem of forced context switching and works well for the most common use cases.

Type 4, the once-in-a-while code review is not a permanent option for a professional team and may be used only to get a team started with code reviews.

Use asynchronous reviews by default

I think that a professional team should have the asynchronous code review in place as the default type because it prevents a lot of drawbacks compared to the synchronous review.

The synchronous review can be used in case the reviewer is not able to make sense of the changes made by the coder. But in such a situation the reviewer is going to ask the coder anyway for additional clarification. And if you work in a team these situations should hardly occur.

In case you don’t have a real team and work as a group of people, then the synchronous code review makes sense. If the reviewer does not know at all what you were working on the last couple of days, then it makes sense to give a proper explanation before you do the review together.

Switching to pair programming makes sense if you have two developers with a similar skill set and work on a complex business problem. But usually a team consists of people with multiple levels of experience and it does not work on complex business problems all the time. Most of the time you have common tasks with average complexity.

Therefore, the best choice for a professional team is to use the asynchronous review by default and switch to the synchronous type or pair programming if necessary.

Ok, that´s it for today.

What type of code review does your team use? Do you know of another type of code review, which I missed here? Then please let me know in the comments.

Talk to you next time. Take care and HabbediEhre.