Are Code Reviews Worth It?

JamaicaBay writes "I'm a development manager, and the other day my boss and I got into an argument over whether it's worth doing code reviews. In my shop we've done both code reviews and design reviews. They are all programmer-led. What we've found is that code reviews take forever and tend to reveal less than good UI-level testing would. The payback on design reviews, meanwhile, is tremendous. Our code is intended for desktop, non-critical use, so I asked my boss to consider whether it was worth spending so much time on examining built code, given our experience not getting much out of it. I'm wondering whether the Slashdot crowd's experience has been similar."

Comments

Yes!

[Omnifarious]

Code reviews have a lot of value in two ways completely independent of how good they are at catching errors. First, they are a way to enforce various stylistic guidelines on code that make future maintenance much easier. Secondly, they are a tool for spreading knowledge about the code around to other members of your development team.

They can also catch some errors that are very hard to catch in any other way. I recently worked on a project in which I found an error that would've caused code to only fail in a very limited and non-obvious set of circumstances. The thing is, somebody would've almost inevitably encountered those circumstances and the phantom and nearly unrepeatable bug reports resulting from this would likely have never been solved.

I fear code ever stepping off into the land of incorrect behavior as there are few corrective mechanisms that will cajole the errant program back into doing something sane. The longer it goes without abject failure, the more weirdly wrong it will be. Therefore I think any and all measures to keep it from ever going there and making sure it dies as quickly as possible if it does are useful.

Depends on what you're looking for

[ipoverscsi]

Design reviews are useful to catch problems early on, particularly the selection of poor algorithms or data structures. However, in the the software shops I've worked nobody does any documentation, which makes it particularly difficult to do a design review. So, as you can imagine, I haven't got much experience with this area.

Code reviews, on the other hand, are more about auditing code to make sure that people are following the coding standards and policies. After all, if you've got coding standards, how are you supposed to tell if anybody is following them without reviewing the code? Once your coding standards have been institutionalized -- that is, most people have internalized them and following them -- then what's the point of a code review?

So your best bet, then, is to reserve code reviews for junior developers until the coding standards are internalized; and use design reviews for the architects.

Re:Synergy, leverage, low hanging fruit, etc..

[piojo]

how many defects are discovered in the review versus how many make it out the door?

I don't think defects are the only metric. Code reviews can result in a cleaner codebase that's easier to understand. Everyone occasionally writes bad code. A reviewer might say, "I see that it works, but I don't like it..." and mention an alternative solution. A reviewer might suggest that something is non-obvious and that a comment is warranted. Code reviews aren't just for bugs, they are to get better code.

Re:Are they worth it?

[Kjella]

There are apparently a couple different kings of things that are both called "code reviews", which one are you talking about? There's also the issue that they're supposedly (as in, according to actual studies) pretty good, so maybe you could do them slightly differently and get much better (more in line with the study results) effects.

Formal reviews is only meaningful if you have an equally formal specification that is unlikely to change often or at all. A lot of heavy backend systems could benefit from that, but this isn't one of them they should definately stop. Of the lighter:

Over-the-shoulder One developer looks over the author's shoulder as the latter walks through the code.
Email pass-around Source code management system emails code to reviewers automatically after checkin is made.
Pair Programming Two authors develop code together at the same workstation, such is common in Extreme Programming.
Tool-assisted code review Authors and reviewers use specialized tools designed for peer code review.

First one nearly never leads to good code in my experience, unless you manage to get just the right mix of writing code and helpful conversation, it's way too easy to zone out, take over, turn it into a lecture or whatever. Second one sounds like SPAM, who reads those? Pair programming can work, but I'm not sure it's worth the overhead.

Tool assisted is definately my favorite. Clone and branch then make your changes and request that they be merged back. You have to say something sensible about what you're doing as a whole, at least two people will look at it, they can comment or reject it. Not according to guidelines or design or whatever? Fix and resubmit. That, together with design meetings I think is the way to go.

Re:Are they worth it?

[man_of_mr_e]

Pair programming can work, but I'm not sure it's worth the overhead

By "overhead" I assume you mean the cost of two developers to write one piece of code. In my experience, Pair programmers are more than twice as productive as a single developer when you factor in all the errors and bugs prevented by having two sets of eyes on the same problem. Of course this only works when you have a pair that can work together, which can be hard to find in some environments.

The other advantage you get from pair programming is that you have two people familiar with the code, not just one. Thus if one leaves the company, or goes on vacation and problem needs to be fixed, you always have another person (at least until both of them leave the company).

Even if you do nothing else that XP advocates, pair programming can really be worth it.

WTF does maybe mean?

[lalena]

Are code reviews useful?
Lets see. Right click -> View source this web page. In the first 10 lines I see a variable called maybe with no comments as to what it means.
Yes, code reviews are useful.