One of questions I always get when talking about code reviews, formal or otherwise, is "What do you think about automatic code review software like CodeCop or varScoper?"

First off I'll say, I love and use varScoper. Second, I like the idea of CodeCop, but I've never been good enough at writing regular expressions to craft my own rules well enough to use it. Those points being said, as great as these tools are, they do not even come close to eliminating the need for formal code reviews.

Automatic testers are great at finding small discreet known issues:

• Did you var scope every variable in ever function?
• Did you make sure not to use relative url's in cfschedule?
• Did you use cfqueryparam on all of your user input driven queries?

But they can't figure out bigger picture issues like:

• Is this function named "computeCosine" actually returning the sine?
• Is there sufficient validation occurring on form posts before they are being processed?
• Is this function doing something that a built in function could be doing better?

In addition to the hard to compute issues above, there are the unknowns. Until someone discovered there was an issue with using relative url's in cfschedule there was no rule written in the code review tool. It takes human eyes to discover these issues, either during a review, or during bug fixing. Only after they are discovered can they be codified.

To put more succinctly and in a metaphorical manner:

Automatic code review tools are spell checkers. Formal code reviewers are editors. Spell checkers and even grammar checkers don't prevent you from writing perfectly spelled and constructed crap. You need an editor to prevent that. Just as code review tools don't prevent you from writing crap code that adheres to all of their rules.

So use automatic tools before a formal code review, just like you spell check an email before you send it out. But don't rely on an automatic tool as your last line of oversight.

Nathan Mische asked a really good set of questions in response to my code review presentation posting. I figured that I would give a longer answer than really works in a comment.

First it's important to point out that formal code reviews are part of a suite of quality assurance techniques. They do not preclude the use of any other, including application design, informal code reviews, testing, etc.

But formal code reviews, a whole bunch of people looking at all of the code, and then having a long meeting discussing it, are relatively expensive. They take a good deal of time from many people who are probably working on their own code, and incurring the cost of context switching by doing so. So my guideline for code reviews is once per major revision of the application. Basically, if you spend a few days doing an update on an application that has already been reviewed, then you probably don't need a formal review. If you and a team of developers spend 4 months doing a revision to the code, a formal review is probably in order. Those are the extreme cases; your organization will have to choose where in between to draw the hard fast line.

As for when to do them… We at Wharton do them basically right before the application moves from development to production. This is also the time when we try the application out on the staging server. The timing is dictated by how we've mandated code reviews. To get your application on our production ColdFusion servers you must go through a code review.

Is that the best time? I'm not really sure. I think if you design correctly, unit test, informally review code, and get user feedback from your clients, then it is the right time to do the review. But we don't always do all of that so I get the subtle feeling that earlier might be better, but we'll just have to deal with the timing forced by our mandate.

Anyone else have any opinions on this?

Back in June, I gave a presentation on formal code reviews to the Philadelphia CFUG.

I've had it ready to share for a while, but I hadn't gotten around to it until now. Enjoy.

Code Review Presentation Given to Philadelphia CFUG