I've been reading through a free book I got a while back - Best Kept Secrets of Peer Code Review - and it's actually a pretty good read. It is a rather dry read, but there's some good information in there. And in case you were wondering, you can get the book for free as well. Just follow the above link.

Anyway, I should note that there's definitely a reason the book is free - it's findings and advice directly correlate with the feature set of their core product - Code Collaborator. Conveniently, there's also a 20-page ad for the software in the book. The software is around $500 per seat, so you can see why it's worth giving away the book. The advice in the book is definitely slanted toward what their own tool does. The question is whether the software was built around the results of the research (of which the book has plenty), or if the research was hand picked to support what their software does. I don't know that answer, but regardless, there's some interesting information in the book - and since it's free, I think it's worth the read.

There are definitely some things I question in the book - for example, it essentially assumes that everyone is doing code reviews and is reviewing every single line of code. That's not something I've ever seen. Regardless of some of those type of issues, there are a few takeaways that I'd love to get a chance to use on my own projects.

To be honest, I haven't been a part of that many code reviews on my projects. When I was at Crowe, we did code reviews on an internal product I was a part of. We didn't review every line - not even close. We reviewed code for key pieces of the system, as well as reviews for new developers. Crowe typically hires (or at least used to hire) a lot of fresh developers - people right out of college - so reviewing their code was important to make sure they were progressing and doing things the way the project expected. At that time, a lot of that was my code. Crowe was my first job out of college. Since we were all relatively inexperienced, we didn't really know much about what we were supposed to be doing. It was more of a walkthrough of the structure of the code rather than a true analysis looking for specific types of issues. Looking back, I don't remember finding many defects in the code, and the ones we did find were more along the lines of code flow and coding standards rather than actual bugs. Still important, but if I had it to do over, I think it would be much more productive.

We've done code reviews on a few of my projects at Sagestone/NuSoft. Based on how we did them, I don't think they were all that productive either, because we didn't have a solid goal for why we were doing reviews. We needed to do a review, so we did one. If I had to pick a goal, it was more to ensure that the code lived up to coding standards, rather than reviewing the code for correctness. It's important to be consistent, but there are easier, less time intensive methods to verify that. Our time would have been better spent looking at the actual logic contained in the code rather than the structure of the code. But hindsight is 20/20, right?

Anyway, there's a few things the book highlights that I found interesting.

• Code reviews don't necessarily have to conclude with a large review meeting. The studies showed that review meetings didn't find a high enough percentage of defects compared to individual reviewers looking at the code on their own ahead of time. The meeting was productive in eliminating false positives - alleged defects that, after clarification, are determined to not actually be defects. But you don't need the whole review team and a formal meeting to obtain the same false positive filtering - you can have the author and one person go over those informally.
• Optimally, a code review should be about an hour and cover 100-300 lines of code. Reviewing the code slowly and methodically is the best way. Longer than an hour though, and the return is diminishing. The majority of defects are found within the first hour of reviewing the code. The optimal amount of code to look at is about 200 lines, because it's small enough to get your head around.
• Use a code review checklist. This now seems so obvious. Reviewers need to have an idea of what to look for. Creating a list of specific types of issues to look for will result in the elimination of those types of issues. This is partly because your reviewers will be looking for it specifically, and partly because the team will stop writing those types of issues. Once they are made aware that it's a problem, they'll be conscientious of it and will eventually adjust accordingly. This means that your list has to be constantly maintained and updated to reflect the current types of issues you're seeing in your software.

That's definitely a shift in how I've done code reviews in the past. For the most part, the prep for the code review itself only consisted of a brief review of the code, and the assumption was that the deep dive into the code would happen in the two hour meeting. It actually makes sense that it should be the opposite - the deep dive should happen ahead of time, and the meeting (if it even needs to happen) should just be a summary of what was already found.

Interesting thoughts, really. But I still wonder how many people are actually doing code reviews. So, are you doing code reviews? Are they effective? What makes them that way?