Wednesday, November 02, 2005

Code Reviews

Ed Gibbs says his team is about to institute code reviews. Of course, if you do pair programming regularly code reviews are pointless, since - turning all the knobs up to 11 - all of the code is reviewed all the time. But I've never worked in a shop where pair programming really took off. I'd be curious to hear how prevalent it is.

As I understand it, we at ProSolv are required by FDA regulations - perhaps here? - to do design and code reviews, although, especially for small projects, we often combine them into a single review. Currently I'm not convinced that they add anything to the quality of our software, although, as I've stated before, I think ISO can potentially be a big gain for a company and not just overhead. All the usual difficulties of code reviews apply - what sorts of things are worth bringing up? Is coder A receptive to constructive criticism? Is coder B tearing things down for the sake of doing it? Is coder C reluctant to make a great suggestion for fear of hurting feelings? Should the code be perfect, or just good enough? - and in the final analysis the review is either marked passed or failed.

I'm sure this process can be improved, but I'm not sure how. Maybe design reviews could be accompanied by UML diagrams. Maybe we just need a big slab of coding standards that have to be applied. For example, a review I'm looking at now introduces two new global variables to a C++ application. I think the industry consensus is that global variables are bad, but certainly the code works. Do we need a coding standard that says to avoid global variables? If we did that, how much extra overhead is added to the process?

I'm seriously considering offering a bounty of ten cents a line for any project that can remove lines of code from an application rather than adding them. I bet that would be more effective than fifty code reviews!

Icerocket tags