Review-time versus Defects-found -- which way does the causality go?
David James, president of Precision V&V Services, writes in with an interesting point about the relationship between "number of defects found" and "amount of time spent in review."
In the Cisco study you showed that a slower review rate correlated with more bugs found. Assuming a constant defect density within the code, that correlation would mean that reviewers should go slower to find more bugs.
But in my experience, bugs cluster, and buggier code requires more reviewer time to find and comment on the bugs. Do you think the causality could actually go both ways? (i.e., careful review finds more bugs, and the presence of bugs slows down the review.)
I think David is right to question the causality that I made perhaps too boldly in the code review book.
To dig into this, it's useful to first break out the extremes. Specifically: If someone takes almost no time to review code, they certainly won't find a lot of defects, so in this extreme the rule is "short review time → not finding bugs." Then there's the case of really buggy code, in which case it's clear that "lots of bugs to address → takes lots of time."
So the causality could be either direction if you take an extreme. Now what about the middle (and by definition the majority)? I think the fact that the causality "crosses over" at some point implies that in middle you will get a bit of both.
Another thing to keep in mind is that data tends to be all over the map; it's hard to find solid correlations. That means it's hard to make strong conclusions about "the middle" for "code in general."
So the practical take-home point is: For reviews with tons of bugs, perhaps they shouldn't enter the conversation about reviews in general. Although there's clearly a cliff after which spending more time on code won't uncover (substantially) more defects, it's also clear that spending too little time means you won't find as many as could be efficiently detected.
After all, a review with "tons of bugs" usually indicates the code is flawed beyond the usual nit-picky stuff. The author might have a funadamental misconception or lack an entire category of experience. It's probably best to do some mentoring at that point -- probably in the form of spot-pair-programming -- to use this chance to teach the author how to not have these kinds of problems in general.
Remember, code review isn't just about cleaning out bugs, it's also about helping each other get better at writing clean code in the first place.