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.
Great post!
I’m with Atlassian, makers of <a href=http://www.atlassian.com/software/crucible/>Crucible</a>. Automated tools make a great compliment to the code review process, but cannot take the place of second pair of eyes interpreting what it is your code is trying to say..
LikeLike