Tuesday 18 December 2018

Are Code Reviews Waste?

I cross posted this post on Medium.

"Waste" is NOT a Dirty Word

My recent talk Agile is a Dirty Word generated a lot of interest at the conference at which it debuted and on social media since. More, in fact, than I ever thought I would get for any of my talks. One of the themes I examined in this talk was waste. This came quite early in the piece and one of the points I was making is that (unlike Agile, Kanban and various others) "Waste" is NOT a dirty word. What I mean by this is that whoever the client is (in my experience at least) and whatever stage of Agile maturity they are at, "Waste" is a concept that people understand and want to reduce.

An "Innocent" Value Stream Mapping Slide

I wanted a way at my client to convey how certain activities don't add value to the business and should therefore be considered to be waste. The slide I came up with for the client was designed to illustrate a very simple value stream map and start a conversation over which activities add value and which are waste. I reused the client slide with only a change to the title in Agile is a DirtyWord. Here is the slide as used in Belgium:


I use the word "innocent" in the title of this section because, as is usually the case, nothing on this slide was ever accidental or innocent. When I originally showed this slide to our client (it has transitions that make parts of it more obvious) I knew that "CODE REVIEW" down in the waste part would be controversial. I knew that several people at our client were obsessed with the idea that code review was an important part of their delivery process. So whilst "code review is waste" was not the title of this slide when I showed it at the client, I very definitely wanted somebody to notice it was flagged as waste and challenge it. This did indeed happen which "coincidentally" let me introduce the subject of why we think code review is waste in the course of talking about something completely different.

What is Waste?

In the rather simplistic model above, I have a notional timeline for a story moving across a wall. We have to assume that the thing that is moving across the wall is valuable. That is to say, I'm assuming that some sensible work by a product owner or similar has taken place and we are confident that when this thing gets released in to production that it will return some value to our company. Note the key point here is that when this thing gets released to production it will return some value to our company - NOT BEFORE. So the clock starts ticking from the point when the story's value statement is articulated and ends when it is live in production. During that time, anything that doesn't help the story move along the wall is waste.

What Outcome is Supported by a Code Review?

One question that we often ask our clients is "what outcome does this process support?" Sometimes you can ask this question of a particularly silly piece of risk management theatre and it can provoke anything from mystification to ultra defensive cat-bear-trap response to complete silence. All three responses, by the way, are a great sign that the process being questioned is unconnected to any real outcome and thus devoid of any value. When it comes to asking this question about code review, however, I've generally received a response along the lines of "code review ensures good code quality" or "we need to check that the code works". So assuming such a response is offered, we can't argue the thing away on the grounds that it can't be linked to a desirable outcome, since "working code" or "code quality" are both outcomes we should be looking for. So why do I argue code review is waste?

What REALLY Happens in a Code Review?

Disclaimer: I am writing this section based on my own personal experience. If anybody has worked in a situation where better things happen with respect to code review I'd love to hear how those better things can be made to happen.

I would also add that all of my comments refer to an organisation where the team is co-located or at most distributed over two sites. I am not considering an entirely distributed team.

The Process

Usually, a code review is requested by a developer when they want to merge some changes into a mainline. This results immediately in the request for code review waiting in a notional queue somewhere for somebody to act upon it. This is undeniably waste, all queuing time is waste, I don't think there is any argument on this one. When somebody "picks up" the code review all they can typically see is the changes made to the code. Depending on the diligence of the reviewer or, more likely, whatever time they perceive they can spare from their "real work", they might fire up their code editor and view the code changes in the wider context of whatever code base it lives. They then annotate the changes (using whatever tool they use) with any suggestions for improvement and update the status of the code review to something like "suggestions for changes made" or whatever the workflow tool in play dictates. At that point the review goes back on the queue of the person who requested the review (more undeniable waste), they react to the suggestions as they see fit and then the cycle repeats until the code reviewer "approves" the changes.

The Comments

Again, I repeat my disclaimer. There are two types of comments that I have seen. The first type are utterly unrelated to anything the code does at all. An example of such a comment would be "We should put opening braces on the same line as a function signature, NOT the line below", or one of my particular favourites "You have used a tab character of width 3, instead of 3 spaces, for your indent" (how did this person even spot that?) These type of comments are not only entirely devoid of value whatsoever, they are irritating, probably passive aggressive (I'm still never quite sure exactly what PA means but I think it applies here) and DEFINITELY should be caught with whatever linting tool is play.

The second type of comment I have seen would be along the lines of "you used a for loop to iterate when you could have used foreach" or "this slightly different way of doing it will be more efficient". Whilst I have slightly more sympathy with this type of comment I still don't think it is worthy of note unless there is specifically something about the implementation that is "wrong" or would be likely to cause some kind of real performance issue.

A Useful Comment

A useful comment could be something like "the tests don't cover all the edge cases" or "There should be an integration test of this thing". I have never seen this happen. Why? I think the answer is that the reviewer lacks the context of the story that was played and therefore lacks the knowledge to make any really useful comments about actual functionality.

Does Code Review Support the Suggested Outcomes?

If we assume that the types of comments that get passed around fall under the not useful categories mentioned above then I struggle to accept that code reviews do anything to support the outcomes of "good code" or "working code" and I would therefore argue that all such process is waste.

What If the Comments are Useful?

This is the tricky part of my argument. I have had some people argue that there are useful comments in the code reviews that happen at their company. As I say, I have never seen this in any review that I've been involved in but I'm prepared to accept that it can (and probably does) happen that some useful comments are sometimes added to code reviews and acted upon by the original developer. Is such a code review waste?

I believe it is still waste. Why? Firstly, the queuing time is there, whatever you do. Secondly, there are at least two context switches involved for different people to support this process. Context switches are expensive and add no value to any process. So they are waste. Thirdly, there is a better solution to the outcome of "good code" and "working code". Both of these outcomes are more easily achieved through pairing or mob programming and you get shared context and therefore better bus factor, into the bargain. Perhaps, if the outcomes actually are being legitimately supported by a code review process then I should downgrade my claim from "code review is waste" to "code review is an anti-pattern". I think that may be semantically more accurate in this case but "waste" is somehow more tangible a thing to eradicate than "anti-pattern".

Can Code Reviews be Value Adding?

I mentioned above that certain code reviews might be arguably an anti-pattern rather than straightforward waste. I'm prepared to accept that they add value if the following things are true:
  • Instead of the workflow, with all the queues mentioned above, the developer requesting the review approaches other developers until she finds somebody available to help with the review.
  • The code review is carried out together between the developer and the reviewer. All suggestions are therefore discussed and collaboratively acted upon.
  • Assuming there isn't the need to do any major extra work (perhaps after sharing the context it becomes apparent that the suggested solution just doesn't do what it is meant to do) the pair will continue until they are satisfied with the result and they will then merge the changes as part of the workflow.
According to the authors of Design Patterns, an anti-pattern has the following characteristics:
  • A commonly used process, structure, or pattern of action that despite initially appearing to be an appropriate and effective response to a problem, has more bad consequences than good ones.
  • Another solution exists that is documented, repeatable, and proven to be effective.
I think I have argued that in general code review fails in its goals and thus fulfills the first part of the definition. I strongly believe that there exists at least two better solutions - pair programming and mob programming. So it is therefore my contention that code review is at best an anti-pattern and at worst (and this is the more likely scenario in my experience) waste.


45 comments:

  1. there are portable wine coolers which also fit in a small office space. i use them in my home office** ReviewsByPeople

    ReplyDelete
  2. Thanks for posting this info. I just want to let you know that I just check out your site and I find it very interesting and informative. I can't wait to read lots of your posts. Junk Car Removal

    ReplyDelete
  3. Hi there, I found your blog via Google while searching for such kinda informative post and your post looks very interesting for me. JUNK REMOVAL SERVICE

    ReplyDelete
  4. I think that I'm into your pieces of writing. I wait for your posts every week. I learn many new interesting things from the articles. The information is essential for me. I want to thank you for sharinghanks for the website article.Really thank you! Significantly obliged.
    junk trash removal austin

    ReplyDelete
  5. I’m curious to find out what blog system you’re utilizing? I’m having some small security problems with my latest website and I’d like to find something more safe. Do you have any suggestions? And Bet Numbers

    ReplyDelete
  6. Your blog provided us with valuable information to work with. Each & every tips of your post are awesome. Thanks a lot for sharing. Keep blogging see this page.

    ReplyDelete
  7. This is a great inspiring article.I am pretty much pleased with your good work.You put really very helpful information. Thank you a lot! and if you need Graphic Design Company then contact us!

    ReplyDelete
  8. On the off chance that the offer is on crude sustenance, at that point you should reconsider in light of the fact that you may need to process them suitably before you can store them. Another choice is to get someone else to pool buys with you so you don't need to manage a capacity crisis. coupon code

    ReplyDelete
  9. These are really amazing and valuable websites you have shared with us. Thanks for the informative post.
    miami dade county services

    ReplyDelete
  10. I’m going to read this. I’ll be sure to come back. thanks for sharing. and also This article gives the light in which we can observe the reality. this is very nice one and gives indepth information. thanks for this nice article... buy google 5 star reviews

    ReplyDelete
  11. Thanks for sharing this information. I really like your blog post very much. You have really shared a informative and interesting blog post with people. soil testing new jersey

    ReplyDelete
  12. Our easy-to-use website and the multitude of custom stickers printing option are just some of the reasons why business, graphic designers, and all our customers trust us with their printing – all backed by our 100% Guarantee!
    Eyyaa

    ReplyDelete
  13. Our easy-to-use website and the multitude of custom stickers printing option are just some of the reasons why business, graphic designers, and all our customers trust us with their printing – all backed by our 100% Guarantee!
    Ey Yaa

    ReplyDelete
  14. I am glad to read this post, it's a good one. I am always looking for quality posts and articles and this is what I found here, if anyone looking to softwarte devlopement service, then visit custom software development company

    ReplyDelete
  15. code as a starting point for your project ... it definitely handles multiple clients. Regarding the message about the 'Task' class ... the 'Task' class is found in the productrapid .Net 4.0 assemblies and should be already referenced from a .Net 4.0 console ap

    ReplyDelete
  16. If you visit other cities and you want to have awesome fun then please meet my friends who are hot and sexy meet these wild beauties for the pleasure you are seeking I assure you that these young women are incredibly sexy and passionate to the degree that you imagine your wildest fantasies will come true hurry up and call them you can know a little about them on this page and then you can choose which one to meet you must grab this opportunity of being with a fine woman with delicate features and romance them so that your life is cherished forever you can go to these towns to have these beauties Watch Series with you and enjoy your life to the fullest just go and book a hotel and then invite these girls to your room for some adventure.

    ReplyDelete
  17. Your blog provided us with valuable information to work with. Each & every tips of your post are awesome. Thanks a lot for sharing. Keep blogging.

    telemedicine app development

    ReplyDelete
  18. I think this is an informative post and it is very useful and knowledgeable. therefore, I would like to thank you for the efforts you have made in writing this article.online marketing firm

    ReplyDelete
  19. Thanks for posting this info. I just want to let you know that I just check out your site and I find it very interesting and informative.
    austin junk removal companies

    ReplyDelete
  20. Hello I am so delighted I located your blog, I really located you by mistake, while I was watching on google for something else, Anyways I am here now and could just like to say thank for a tremendous post and a all round entertaining website. Please do keep up the great work. logo packages

    ReplyDelete
  21. Wow! Such an amazing and helpful post this is. I really really love it. It's so good and so awesome. I am just amazed. I hope that you continue to do your work like this in the future also seo-ranking

    ReplyDelete
  22. I dont think so that such code reviews are waste. This is because I have seen such codes being implemented in a graphic design contest where graphic and web designers were taking part.

    ReplyDelete
  23. This is a wonderful article, Given so much info in it, These type of articles keeps the users interest in the website, and keep on sharing more ... good luck. someone write my essay

    ReplyDelete
  24. I have perused your article, it is exceptionally enlightening and helpful for me.I respect the profitable information you offer in your articles. Thanks for posting it. Browns vs Ravens Live Stream

    ReplyDelete
  25. Such an enlightening website. An obligation of appreciation is all together for sharing. Furniture is a thing that we in general need in our home or workplace. In case you thinking about obtaining new goods, by then I propose you buy victorian decorations. Since it's altogether pleasing and the arrangement is so impeccable. dallas cowboys streaming live

    ReplyDelete
  26. ?m impressed, I must say. Really rarely do I encounter a blog that?s both educative and entertaining, and let me tell you, you have hit the nail on the head. Your idea is outstanding; the issue is something that not enough people are speaking intelligently about. I am very happy that I stumbled across this in my search for something relating to this.Get Best FR Fabric of ISO 11612 from Daletec.com in all over the world.

    ReplyDelete
  27. You have done a great job. I will definitely dig it and personally recommend to my friends. I am confident they will be benefited from this site. watch patriots live online

    ReplyDelete
  28. buy targeted traffic for ecommerce or affiliate website. Best place to buy real human targeted traffic. 100% Guaranteed Real Targeted Visitors. ➤Targeted traffic that converts. Buy website traffic, buy targeted traffic and increase web site traffic.

    ReplyDelete
  29. Super site! I am Loving it!! Will return once more. I'm taking your sustenance in addition. Thanks how to watch nfl games live online

    ReplyDelete
  30. Excellently written article, if only all blogger offered the same level of content as you, the internet would be a much better place. Please keep it up!.Great tips, I would like to join your blog anyway.Waiting for some more review.Thank you
    junk removal on long island

    ReplyDelete
  31. I am upbeat to locate your recognized method for composing the post. Presently you make it simple for me to comprehend and execute the idea. Much obliged to you for the post. business listing sites

    ReplyDelete
  32. Awesome dispatch! I am indeed getting apt to over this info, is truly neighborly my buddy. Likewise fantastic blog here among many of the costly info you acquire. Reserve up the beneficial process you are doing here. cursos de ti online

    ReplyDelete
  33. Good day! I simply would like to give you a huge thumbs up for your great information you have here on this post. I am returning to your website for more soon.
    Tech PC

    ReplyDelete
  34. شرکت تلکا هاست در تلاش است تا در خدمات خود بهترین باشد و در زمینه هاست فوق ارزان همواره کمک به وبمستران عزیز بکند.همچنین تلکا هاست در هاست فوق ارزان و خرید هاست دانلود ارزان نیز فعالیت دارد و هاست پرسرعت ارزان هم دارد.

    ReplyDelete
  35. Positive site, where did u come up with the information on this posting? I'm pleased I discovered it though, ill be checking back soon to find out what additional posts you include. كود خصم نمشي

    ReplyDelete
  36. This is also a very good post which I really enjoyed reading. It is not every day that I have the possibility to see something like this.. review

    ReplyDelete