David R. MacIver's Blog

Review of a book that reviews (code) reviewing

In an earlier version of my recent software development advice post, I said the following:

I also think this will not significantly reduce the number of bugs in your software. Code review is just not a cost effective bug finding tool compared to almost anything else.


I appear to be wrong in this claim, if the literature is to be believed. I don’t have a strong reason to doubt the literature, and on examination I’m not actually certain why I believed the contrary (it appears to be cached knowledge), so I’m going to change my opinion on this: Code review is probably a very cost effective way of finding bugs if you do it well, and may be reasonable at it even if you do it badly.

The thing that’s updated my opinion on the subject is reading “Best Kept Secrets of Peer Code Review”, after Richard Barrell sent me a link to a chapter from the middle of it reviewing the literature.

With a couple caveats (the title being terrible definitely counted amongst them), the book is pretty good. There are two chapters worth skipping, but that’s more a function of the fact that the book is from 2006 than anything else (GitHub’s pull requests, while far from a stellar code review system, are in all ways better than what was widely available back in 2006 and for the common path probably not much worse than the state of the art).

The authors clearly have an agenda and the book is there to promote that, but that’s OK. I have an agenda too - it’s very hard to have expertise without also having an agenda in support of that expertise. And even if I believe they’re overstating the case, the case they present is reasonably convincing. It’s also refreshingly empirical - most software engineering advice is full of anecdotes and opinions and, while this book absolutely contains some of those, it’s also got a really pleasant amount of actual research backed by actual data. The data is often not of amazing quality in that many of the studies are quite small scale so the statistics are likely under powered, but that’s still a better standard of evidence than I’m used to.

It’s also pleasantly short. The whole book is only around 160 pages, and I read it over the course of an afternoon.

Anyway, that’s the review section of this post done. Now it’s time for the review section.

I have a couple take homes from this book which are currently in the state of “Plausible sounding statements that I will think about further and probably upgrade most of to actual beliefs”.


The checklist one is interesting, and I am actually pretty convinced by it: The overall literature on checklists helping construct reliable processes seems to be reasonably convincing (but my opinion here is very trend driven and I have not actually read any primary research on this subject. It conforms to my biases about the world though, and thus must be true), and the specific data presented in favour of checklists for reviewing is moderately convincing. This is the advice that is the most likely to make an actual practical difference to how I review code in future.

The advice on checklists is interesting in particular in how it’s constructed: They strongly recommend not having just a “standard” checklist, but actively curating one over time: When something is found in review that seems like it would make a good check list item, add it to the list! When an item hasn’t found anything useful in review in a while (say 6 months), think about removing it.

Here are a couple example check list items from the book that look generally useful to me (the example list this is from is much longer):


There are plenty more. The book suggests that checklists should really only have about the top 10-20 items that you’ve found most useful over time (the sample checklist has 25 items, so apparently they didn’t test the invalid input case here).

One interesting thing worth highlighting is that at least some checklist items may be worth automating out of the review entirely. e.g. “Testing: Unit tests are added for new code paths or behaviours” would mostly be better handled by enforcing a code coverage metric I think.

As well as improving the quality of the review itself, the book highlights another interesting application of checklists: It doesn’t have to just be the reviewer who goes over them! Doing a sort of self review by following the checklist yourself before submitting seems to be nearly as effective at removing bugs as the review itself would be (it is unclear to me if that remains true if you skip the review altogether - my feeling is that people are likely to get sloppy at the checklist if they imagine someone is not going to be checking it later). This presumably speeds up the review process a lot by reducing the number of back and forth exchanges required to pass it.

One suggestion off the back of this self review that they made (which seems to come from the Personal Software Process world) is that as well as maintaining a global checklist for reviews it might be worth maintaining individual checklists for reviews, where people maintain their own list of things that it’s worth them pre-checking because they often make a mistake in that area which then comes up in review.

Anyway, I’m really glad I read the book. I can’t swear to its correctness, but it’s an interesting and plausible perspective that I’ll be thinking about further.

PS. If you’d like to fuel my book habit, I have a public Amazon wishlist. I’ll happily review any books you buy me...