What is it all about?
Code review is one of the engineering practices that in my experience from time to time is neglected by the team or individual developers. Some complain that it eats up a lot of time and bugs still get through the process, some said that mandatory code review could block you for a long period while you are fixing critical production bug and literally could cost a lot of money to the company. Some developers remember conflicts with other developers over some minor things in their past code reviews which stopped useful work for a while.
There are tons of articles about code reviews best practices, and several tools to help you with it. In the article we will not discuss how exactly perform code reviews, instead, the main question is do you even need code reviews in your organization or team? The answer to this as usual in software development — it depends. In the next article, I plan to discuss how the process of code reviews could look like, and what the differences between existing processes are.
Let us first discuss the arguments for needing the code review and against it.
Cons of code reviews
The cons of code reviews are easy to see especially when there is some process of code review in the team, but it is not well documented, or works differently from time to time. Some of the cons are:
Code reviews cost the engineer’s time. One (or more) engineers already spent their time to implement something (new feature, fix the bug, refactoring, etc.). Why do we need to spend the time of another engineer (or more than one) to read and comment on it? Moreover, most likely the first engineer will have to spend time fixing whatever the reviewer found unsatisfactory and since it is an iterative process it likely will more than once happen until the change will be submitted. This led us also to another question, should code reviews be performed before code is submitted or after? Should this process be blocking for any code before it will be sent to the repository? We will discuss it in the next part, which will be about the code review processes.
Related to the first one. Code review slows the development. Like any process that can block further progress, code reviews could cause features to be delayed, the bug is unfixed longer, and will negatively impact the team speed. Some of the code review processes are not blocking, however, they are still costs engineering time (see p.1) and thus cause the speed reduction.
Again related to the first one. Code reviews are switching context for the reviewers, and in general, more context switching for developers tends to decrease their performance (see 1, 2, or 3 for details). This even further affects engineers’ productivity.
Mandatory code review could block your super urgent production bug fix for a while and the company will lose the money while you are desperately trying to call to other developer, which should perform code review, but he is on lunch and forgot the phone on the desk.
Code reviews could initiate conflicts between engineers. Sometimes engineers could think that the comments to their code are personal attacks on them. Sometimes it could be the attack. Sometimes the series of comments and replies could lead to long debates and conflicts and eat up even more time. Sometimes the fact itself that less experienced engineers will review their code for example could cause the anger of a senior engineer. Obligatory relevant xkcd included.
There are also other cons, however in practice the points I listed above are the main arguments against code reviews. As we understand from that list, the main concerns are related to the time, which is spent on this process and on conflicts, which it could initiate. There are of course methods to mitigate most of these concerns, however, it is inevitable, that code reviews will cost at least some percent of developers time. The detailed discussion on how to mitigate the negative impact of code reviews logically belongs to the discussion of how to organize the process of performing them and will be placed in the next article accordingly.
Pros of code reviews
Now, what you gain for it? The pros of code reviews:
Finding Defects. This is often considered the main reason and profit from code reviews; however, the study shows that this aspect is usually overstated. The actual number of defects missed during code reviews is greater than many people expect. Still, some defects will be found on code review and this will be defects that were missed by unit tests. You wrote unit tests before the review, don’t you?
Code improvements. This is one of the important profits. If you care about the code quality, (we will discuss later cases when this could be less important) this would be one of the main motivators. Code reviews help maintain high code standards. For example, Google has the “readability” thing (some explanation here) which is internal certification, which states that you understand Google best practices to use some particular language. Every change in code before committed should be reviewed by someone who has readability in the programming language change was implemented. It could be skipped only if the author of change has the readability. In that language. To get readability one should submit quite a few code changes that are reviewed by the committee of developers that already have readability and they decide when you are ready. It usually takes a few thousand lines of code changed cumulatively to get the readability. This approach adds some guarantees that the code changes will follow the best practices and the codebase will not have quality degradation over time.
Alternative approaches. This is sometimes mentioned as one of the important points. However, in my opinion, if there is some need for an alternative approach discussion after the initial implementation, often this means that there are some problems in the process. The correct place for the approach discussions is the design review, which should precede any implementation. In this case, any alternative approaches on this stage should be small enough to fit in the code improvements part.
Knowledge transfer / Share code ownership. I combine these two because they are strongly related to each other. When more than one developer familiar with the code (and code reviews are lead to this) it increases the “bus factor”. The bus factor is the famous metric, which shows the number of persons that could suddenly disappear that will not disrupt project development. A low bus factor could lead to significant problems in a case when some key developers will suddenly leave, so it is generally advised to keep it high.
Increase learning / Mentoring. This aspect is especially important for new team members as well as for the junior developers. Performing and receiving code reviews helps them ramp up faster and become familiar with the team processes. From my experience code reviews with the new or junior team members is also the most significant source of changes in documents like code review convention or new team member ramp-up doc. That is because of the increased number of questions from these team members that could help to find gaps in existing documentation.
So do you need code reviews?
Now we discussed the pros and cons of code reviews, and next, we should have some framework for how to decide if we should use it.
There was some researches performed in the past, which are tried to find out the gains that code reviews provide and if they are worth it. The exact calculations of profits and losses of code reviews are hard to perform since many factors affect the productivity of the team, and code reviews have different aspects, which are increase or decrease the overall results. Therefore, these papers (for example 1, 2) uses developers’ and managers’ surveys as the main method to get the data for analysis. We will discuss a few examples and see if there is a point to use code reviews.
There is a famous saying that code is written once and then read many times. I am not sure about the original author but looks like it was Robert Martin in the book Clean Code. This and another fact, that better-written code, will cost less time on reading, leads us to the conclusion that over the lifetime of the code we will save time during reading this code multiple times. A lot or probably most big software companies utilize code reviews in their development processes, which means that they believe that code reviews cost less than they bring to the table.
Let us go back to the Google example. Google code review practices said that every (with a small set of partial exclusions) code change should be reviewed at least by:
- Someone who is not the author of the code,
- Someone, who is the owner of the part of the code is changed, and
- Someone, who has readability in the programming language in which the change is implemented.
Note, that some or all the roles could be combined in one person, but there should be at least one person, who is not the author who looked at the code before it could be submitted.
This is Google, the company famous for its high hiring bar (if you are unfamiliar with the term “hiring bar”, check here. Also, it looks like I do not have a relevant link that proves that Google has a high hiring bar, but my personal experience and few more cases that are anecdotal lead me to this evidence). All these developers and the best of these developers are passing code reviews for every change in codebase they make. Compare it with cases when a senior developer in some organization says that he will not send any code on review because he is most senior here and such there is no value in code review. I saw dialogues like that.
There are cases when the code reviews are not necessarily the best approach though. We will discuss it in the following paragraph.
Cases When you probably do not need the code review.
The main reason for the decision to not include code reviews in the development process is when you know exactly that the code is not going to be read in the future.
It could be the prototype you implementing to prove some concept. In case of success, you will rewrite it from scratch to the final version (and not will try to make a stable system from this prototype). Alternatively, it could be a static code. For example, some hardware-related code will be written once and never be changed in the future. In this case, it is not so useful to increase the quality of code for reading, but other profits from code reviews still should be considered.
Also, there is an existing excuse to not perform code reviews in the team. In cases when the whole project is implemented by a single person. In this case, it could be hard to find someone motivated enough to read the entire project’s code so it is probably impractical. However, there are approaches to this situation as well. Some discussed here.
So, in which situations you will get benefits from code reviews and thus should perform them? In most other cases. Especially in cases when you are building software that will run for years.
In the next part of the article, we will discuss the best approaches to organize the process of code reviews.
Links:
- https://www.microsoft.com/en-us/research/wp-content/uploads/2016/02/ICSE202013-codereview.pdf — Microsoft paper Expectations, Outcomes, and Challenges Of Modern Code Review.
- https://web.archive.org/web/20150428192217/http://csalpha.ist.unomaha.edu/~hsiy/research/sm.pdf — Does The Modern Code Inspection Have Value? Bell Laboratories paper on the value of code reviews.
- https://github.com/google/eng-practices/blob/master/review/index.md — Google guide to code reviews.
- https://blog.fullstory.com/what-we-learned-from-google-code-reviews-arent-just-for-catching-bugs/ — Former Google engineers’ takes on code reviews.
- https://softwareengineering.stackexchange.com/questions/139321/how-do-i-review-my-own-code — Discussion of how to approach a situation where there is no other person to review the code.
- https://www.linkedin.com/pulse/context-switching-developers-paul-graham — How Context Switching destroys Developers Productivity and how to fix it.
- https://simpleprogrammer.com/context-switching — What Is Context Switching and How Does It Demotivate Programmers?
- https://arxiv.org/pdf/1805.05504.pdf — Two Sides of the Same Coin: Software Developers’ Perceptions of Task Switching and Task Interruption
- https://www.pullrequest.com/blog/google-code-review-readability-certification — Getting the Certification to Review Code at Google
- https://en.wikipedia.org/wiki/Bus_factor — Bus factor article on Wikipedia.
- https://xkcd.com/1833/ — xkcd comic strip “Code Quality 3”.
- http://blog.interviewing.io/the-eng-hiring-bar-what-the-hell-is-it/ — Hiring bar discussion.
Top comments (0)