Codementor Events

You should stop doing code reviews right now

Published Oct 31, 2018Last updated Apr 28, 2019
You should stop doing code reviews right now

This post is parody -- more people than I expected took me seriously, so unfortunately I have to spoil some of the fun by moving the disclaimer to the top.

Does your team do code reviews? If yes, this is hurting you, your team, and ultimately your business. You need to stop everything right now and get your team to cease and desist with this cancerous practice as soon as possible.

If you are not yet doing code reviews, it has almost certainly been suggested a few times that you start. DO NOT DO THIS. Although people will come up with a million and one reasons why you should start doing code reviews, all of these reasons are flawed, and once you start doing code reviews it can be difficult to stop.

Nothing harms dev teams more than code reviews. I have seen code reviews cause havoc wherever technology is built. Large companies, small companies, cloud providers, machine learning consultancies, academia, and financial corporations -- so don't come here with your "it depends on the situation" line. CODE REVIEWS ARE BAD. It doesn't matter what team size or set up you have, code reviews will hurt you.

Recently, I had an argument where my team refused to accept my authority or superior experience. I had to resort to spelling out my arguments to get them to accept how abhorrent any engineering process built around code reviews was. To help others in a similar predicament, I outline some of the biggest harms code review can cause below -- feel free to share this article with dev teams that you care about and use it to argue that code review practice should be abolished globally.

For each "code review harm", I present a specific example from my experience working in developer teams and then show the general principle that can be drawn from that example. I have fictionalised names and companies to protect people's reputations, but these examples should sound familiar to anyone who has worked in the software development industry.

Code reviews harm experts and weaken job security

At LargeCorp, Jim was one of the senior developers. He had written most of the payments system from scratch and he had maintained it for five years. All changes, updates, patches, and fixes were made by Jim because he knew that system like the back of his hand. Jim was the payments expert at LargeCorp and alongside his duties of maintaining and extending the payments integrations, he also had a full plate of communicating internally to the Sales and Customer Support teams. The system was old and had some quirks -- for example, on every second Friday of months which had 30 days, customers had to add an extra 0 to the amount that they wanted to pay. Somehow Sales and Customer Support always forgot this, but they knew that they had to call up Jim whenever something weird happened with payments, and then he would patiently remind them what needed to be done to make the system function as expected.

Jim had a fairly unique coding style, and because he was busy extending the system to new payment providers while also helping others use the system, he didn't have time to write documentation. This meant that no other developers could come even close to matching Jim in terms of efficiency when it came to changing payments --- changes made by any other dev took three times as long, and fixes (especially emergency fixes) often took longer by a factor of ten.

Jim was the de-facto payments expert at LargeCorp. He recieved regular raises to keep him around and he was well known as the guy who could answer any question relating to payments.

One day, Jim's house got broken into and his laptop was stolen. By chance, the payments system went down shortly afterwards, and customers were unable to spend money on LargeCorp's website until IT had organised a new laptop for Jim, he had set up his environment (which took several hours -- experts are picky about things being just so), and had been able to develop and push a fix. Management realised that things could potentially go very badly if anything happened to Jim, so they organised corporate accommodation and private security guards to accompany Jim wherever he went, ensuring his safety and by extension the long-term health of the company.

Jim was an expert, and his perks and job security were well earned. Unfortunately some younger devs at LargeCorp caught onto the code review trend and tricked management into agreeing that the team should experiment with code reviews. Worse still, Jim's application to be exempted from code reviews was rejected (yadda yadda 'team buy-in' -- I did not at that stage have the authority needed to support Jim's application for an exemption).

Not only did Jim waste valuable time having his code reviewed and reviewing others' code, but this also led to some other developers becoming better acquainted with how the payments system worked. Every time Jim made any changes or integrated a new system, other developers would spend significant time reading code relating to payments. Soon three other developers were able to make changes and fixes to the payments code. Jim's expertise was soon no longer as valuable to the company and due to his far-higher-than-average compensation, he was let go in a round of budget cuts shortly after.

This is one example of how code reviews can devalue experts' knowledge by spreading it too broadly within a team, and it is by no means an isolated one!

I have similarly seen entire teams threatened by this phenomenon. In fact, after Jim left and we had three developers who were able to take on his responsibilities, while keeping their old ones, our operations team was suddenly relocated into sales. It turned out that they had spent most of their time operating an emergency manual payments system, which was used every time Jim's system went down, and which relied on a complicated set of spreadsheets. Because the three developers quickly removed a lot of the quirks from Jim's system and set up a rotating on-call roster, the manual payments system was needed far less often. One of the ops guys hated sales and he left, so code reviews effectively lost us two employees and upstaged an entire team.

Code Review Harm 1: Code Reviews harm experts. The practice devalues expertise, spreads knowledge too broadly, and reduces job security for your most valuable developers. You risk losing good developers either after their expertise is devalued, or even before, as developers are often clever enough to predict being made redundant in advance of it actually happening, potentially leaving you in a sticky situation of having knowledge black holes. Team stability and processes are similarly threatened as broader knowledge can lead to more suggested changes, which results in a lack of overall stability.

Code reviews upset hierarchies and chains of authority

A more personal example: while I was working as the lead dev at PropertyCorp, I did not yet know the full extent of the damage that code reviews could cause. Before I joined, the team had already fallen into the code review trap, and I hadn't yet managed to get them to abandon the horrible practice.

One summer, we had an intern developer, Sam, join my team, on a break from university. Luckily I did not have to interact with this intern much, as we had an intermediate developer on the team who acted as the intern's buddy, taking care of her constant questions.

Although we had a strict hierarchy in place of who could approve code, the actual code reviews themselves were visible to everyone in the team. I had been working hard for days on a new feature against very tight timelines and on a Friday afternoon I had finally completed it. Beer o'clock had already started, and John, the senior dev who was assigned to review my code, was already slightly tipsy. He commented with a standard "LGTM" (looks good to me) and I was about to push the feature live (it was needed by Marketing first thing Monday morning), when Sam shyly walked up to me and said, "I was looking through some code reviews to get more familiar with the code base, and I think I noticed a bug in your feature. I know I'm not assigned to review this, but I think that when you clean up the temporary files, there are some cases where you delete all files from the entire hard drive instead of just the ones that your widget created".

Sam pointed out the how the code that was meant to execute rm -rf /tmp/widget/* could in some edge cases (well, whenever the code was executed on any machine except for my local developer machine) instead run rm -rf /.

Although Sam was right in this case, and her noticing the issue saved production from going down on a Friday evening, this actually caused more harm than good for several reasons.

  • Sam's ego was obviously inflated by the incident. She gained far too much confidence for an intern and started communicating more often with all of the other developers after that, often having the gall to review their code and leave comments where she thought there was room for improvement.
  • Conversely, my ego suffered. I doubted my own technical ability for days afterwards, resulting in reduced productivity as I spent more time checking my own code for bugs before pushing it live, even though it was meant to be John who reviewed my code.
  • The entire team found the incident highly amusing, and laughed about it again on the following Friday. This made me feel that my authority was undermined, leading to another drop in productivity for me and by extension the team.

Code Review Harm 2: Code Reviews upset natural hierarchies and can lead to the mistaken impression that junior devs can find mistakes in the work of highly experienced developers. Although this is sometimes true, the harm done in reducing respect for authority and having underlings treat their superiors as peers far outweighs the benefits of finding a few bugs.

Code reviews confuse developers about their role

Traditionally, developers pay a lot of money to go through a university computer science programme and learn how to do software engineering. Then they get paid a lot of money to do software engineering. This is the natural order of most professions. My next example will show how code reviews can upset these clean and logical economic systems of value exchange.

While I was working at UpStarz, a young and vibrant start up, I noticed that the developers there expected to continue learning while at work. I was flabbergasted -- learning is something that you have to pay for. For many, learning is a valuable, fun, and overall desirable activity, which is why people are willing to pay $100k+ for the university experience. After finishing university or running out of money, people get a job where they get paid to produce output. Obvious, right? Apparently not to the youngsters at UpStarz, who had demanded -- and received -- a book allowance, time to take online courses, and in-team "mentorship".

I wondered where these privileged, entitled kids had got such outlandish ideas, and spent some time talking to staff who had been at UpStarz since the start. Eventually, I traced these ideas back to -- you guessed it -- code reviews.

UpStarz had one of the most tenacious code review habits I have ever encountered. Developers had even learned to enjoy reviewing each other's code. Instead of merely signing off on code reviews or tersely pointing out issues identified during a review, developers would habitually leave long and conversational comments for each other. One of the examples was so bad, that I saved it for future reference, and I'm glad to have an opportunity to use it now. I've reproduced it verbatim below:

It looks like you're hitting the database more often than necessary here, as you're making database requests inside the for loop. Instead of looping through all users, checking each user to see if they qualify for a discount, and then fetching that user from the databse, you should use a filter() statement right above the loop to fetch only the users you need. This will result in only one database call instead of ca. 120, and move most of the required processing over to the database itself, which is optimized for exactly this kind of call, leaving the web server freer to respond to increased traffic over the busy Black Friday period.

If you're not sure how to do this, you can read about Django filters in the official documentation, which is surprisingly easy to read and has come great examples.

If you have time, also check out this really nice article on optimizing performance issues when using Django's ORM.

Can you believe it? A senior engineer actually wasted significant company time on not only reviewing this developer's code, but also on finding relevant educational resources, and putting together a long-winded explanation, with irrelevant business context, on what the problem was. Obviously, the senior engineer was being paid by the company, and should have simply made the optimization changes himself as this would have been far faster. Not only was this code review a once-off waste of time, but the junior developer worsened things by telling the senior developer how much he appreciated the assistance, which encouraged the senior developer to continue leaving detailed code reviews from then on.

One thing led to another, and soon the whole team was hooked on this free education drug. Instead of going back to university or purchasing training programmes and courses, they came to expect that they would continue to be paid by their company to learn, and soon they were spending more than 20% of their time actively "learning on the job" instead of spending their time focussed on producing code to meet their employer's needs.

Code Review Harm 3: Code reviews confuse developers about their roles and goals. They potentially upset standard economic systems built around giving value in return for value, and instead shift expectations, sometimes to the extent that developers feel entitled to be "double paid", both in money and in education.

Code reviews reduce accountability

Remember Jim? The guy who wrote all the payments code at LargeCorp? Well my favourite part about working there was that I always knew exactly who to blame when things went wrong. Did payments break? Call up Jim and shout at him. Simple.

Well code reviews spread responsibility and accountability in a dangerous way. After code reviews were introduced at SecCo, I sometimes wasn't sure who to shout at when things went wrong. Was it the author's fault for writing bad code, or the reviewer's for failing to identify it? I could simply still blame the author of the code, but having had another person take a look at it before going live did make the author feel less responsible, and I often found my shouting had less of an effect. Developers who have made code changes by themselves feel responsible when it breaks and they are willing to get up at 3 a.m. if necessary to fix it.

Developers who are used to code reviews become strangly confident and detached. It's not uncommon for them to assume that the reviewer is partially responsible for catching their stupid mistakes, and I've even had developers who refuse to take any responsibility at all, claiming that "the process" should be responsible for catching errors instead of people. They say it's "only human" to make mistakes and that systems built around teams should account for this. Bullshit! I can't call up a process or system at 3 a.m. and let it have a piece of my mind. People make mistakes, and so people should take responsibilty for fixing them. This builds character and makes people more careful and less likely to make mistakes in future.

Code Review Harm 4: Code reviews deflect responsibility and blame away from individuals and onto processes. These individuals then argue that mistakes are to be expected and are no longer scared of the consequences of making a mistake.

Code reviews make a code base boring

This is not a specific example, but each developer I've worked with has a unique creative style. Dev teams that don't do code review allow individual developers to maintain their own style, and to fully express themselves creatively, allowing their personal flourishes to diverge more and more from a boring norm.

However, the first thing that often happens when code reviews are introduced is that developer styles start clashing. Megan wants to do things one way and Jack wants them done differently. One dev wants tabs, another spaces; half of them keep braces on the same line as loops and the other half devote entire lines to a single brace; no one can agree about variable naming, line length, vertical spacing, or a million and one other personal stylistic choices. Often, code reviews are the thing that force a company to adopt the hated "style guide" -- a soul crushing manual that dictates how code throughout the company should look and feel.

Suddenly, all the developers are coding in the same way. All the code looks the same. All individual style has died. I've seen developers who were so strongly attached to to their style leave a company when asked to comply with a style guide.

Code Review Harm 5: Code reviews kill individual style and creativity, and this can cause a talent exodus, not to mention a dry and boring code base.

Code reviews slow dev teams down and make developers less productive

The last point is almost too obvious to mention, but it's harmful enough that it should be made explicit. BorIn, a traditional corporate where I spent a few years, had the most detailed developer productivity metrics I've ever seen. They also introduced code review shortly before I joined and I ran the the first investigation into the effect of code review on developer productivity. No surprises, there was a huge drop in productivity on all scores at exactly the same time that code review was introduced. Let's look at some concrete figures and see how this happened.

Before code reviews, developers at BorIn were writing, on average, 300 lines of code per day. After code reviews were introduced, this dropped to 100. Yes, code reviews dropped developer productivity to 33% of their potential, and if BorIn hadn't had such accurate metrics or if they hadn't brought me in to help analyse the results, they might still be doing code reviews today. So how exactly do code reviews harm developer productivity by such a large margin? Let's look at how many interactions code reviews can add to a single change.

Before code reviews:

Alex:

  • takes a ticket from the Jira board
  • spends the day writing code
  • ships the change

Total interactions = 3

After code reviews:

Alex:

  • takes a ticket from the Jira board
  • spends the day writing code
  • looks over the code and wonders what the reviewer might suggest, and decides to come back to it tomorrow
  • makes some optimizations and changes without guidance, to try and cut down actual review time as much as possible
  • submits code to a version control system
  • fills out a code review template, including a description of the code change, a link to the ticket, an explanation of why the change is necessary, what was done to test it, a note about the weird edge case that was discovered but not fixed as it wasn't part of the ticket, and a tag for the reviewer who has the most experience with this area of the code base
  • realises he hasn't actually run one of the tests that was ticked off in the review template
  • goes back to environment and runs the final test
  • notices an error from the test and fixes that
  • pushes the code to version control again
  • submits the code review request
  • works on some related documentation that was incomplete while waiting for the review to come back again

Bobbie:

  • gets the code review request notification
  • looks at the code changes
  • comments on three places where the code could be improved
  • submits the review
  • goes to look at the documentation after realising that that bit of the system hasn't changed in a while and isn't well understood by most people

Alex:

  • reads Bobbie's comments
  • makes two of the three requested changes
  • responds to Bobbie about why the last change looks necessary but actually isn't
  • adds a comment to the code to explain to future developers why the third change suggested by Bobbie might look like the right way of doing things, but actually isn't
  • pushes the code to version control yet again
  • finishes updating the documentation while waiting for another response from Bobbie

Bobbie:

  • looks at the two code amendments and one comment left by Alex
  • resolves the comment and approves the code change

Alex:

  • deploys the change

Total interactions = 25+

Laid out like this, it's a miracle that the productivity drop was only 66%! We can see that we are wasting significant time from Alex and Bobbie, and because Alex doesn't want to context switch to a new ticket before resolving the first one, more time is wasted on unimportant tasks like updating documentation, which is so unimportant that it's not even tracked by the developer productivity metrics.

In the worst case I saw at BorIn, a developer pointed out in a code review that a third-party library could be used instead of the 100+ lines of code contained in the code change. The author went and deleted nearly a day's worth of work and spent more time reading the documentation needed to integrate the library. It was the first and hopefully only time in my career I saw a developer with negative productivity metrics -- over that month, the same dev methodically went and used the third-party library in dozens of similar parts of the code base, deleting hundreds of lines of code in each instance, and finally finished the month having a higher count for lines-of-code-deleted than for lines-of-code-written.

Code Review Harm 6: Code reviews slow down your developers and distract them from what should be their only goal: producing lines of code as fast as possible. Code reviews waste the time of at least two people per review (the developer and the reviewer), and there is more time lost while the developer and reviewer wait for comments from each other. In some scenarios, code reviewers can encourage code authors to delete code, reslting in very low or even negative producitivity metrics.

What you can do

In summary, I feel pretty strongly about code review. If you got this far without working out that this entire piece is satire, you're not alone (though I do hope that you're part of a small group). The fact that it is not more obvious that this is satirical is a pretty big problem in the world of software development.

If you work with code, are interested in code review, or just want to share that some of the above sounded familiar to you, please do feel free to ping me at any time on Twitter or by using my first name at my last name .co.za (not .com).

Discover and read more posts from Gareth Dwyer
get started
post commentsBe the first to share your opinion
Show more replies