Break Points
Auditing Code Inspections
Jack Ganssle
6/14/2010 1:58 AM EDT
But inspections suck. Few developers really enjoy them. I sure don’t, but we’re professionals hired to build systems as efficiently and as perfectly as possible. A doctor might not enjoy giving prostate exams much, but to avoid such tests just out of personal distaste is both unprofessional and unacceptable. The same goes for the unsavory bits of our work.
Because of this common aversion it’s not uncommon for a team that starts with all of the best intentions to slowly find reasons to avoid reviewing code; that nearly always eventually results in inspections becoming completely abandoned.
Because of this I feel inspections are one of the few areas where Genghis Khan software management is appropriate. A strong leader insists on their continued use. That person both supplies the resources needed and audits the process to insure compliance.
But you don’t want management on the inspection team (developers tend to avoid pointing out errors when the boss is around, out of respect for their colleagues). So how can one insure the team is doing the right things?
The solution is root cause analysis, used statistically. From time to time the boss should identify a bug that was found and perhaps fixed, and have the team or even a member of the team figure out how the mistake made it into the code.
Was an inspection on the affected code ever done? How much time was devoted to it? Were other problems identified? (This implies collecting metrics, which is nearly painless when automated with tools like SmartBear’s Code Collaborator). It’s possible the function was unusually complex and the review did find many other problems. So, were complexity metrics taken?
Or – gasp – did the developers shortchange the inspection or skip it altogether?
Perhaps the bug slipped in later, post-inspection, due to poor change management procedures.
Bugs are part of the process, but proper bug management should be an equally important component. It’s not possible to do a root-cause analysis of every, or even most, problems. But some level of it keeps the developers on the proper track and can identify flaws in the system that cause delays and quality problems.
C.A.R. Hoare said: “The real value of tests is not that they detect bugs in the code but that they detect inadequacies in the methods, concentration, and skills of those who design and produce the code.” And that observation is also true for looking for root causes of at least some of the bugs.
Jack G. Ganssle is a lecturer and consultant on embedded development issues. He conducts seminars on embedded systems and helps companies with their embedded challenges. Contact him at jack@ganssle.com. His website is www.ganssle.com.





krwada
6/14/2010 1:15 PM EDT
Actually, code inspections are OK. However, the process as we know it definitely sucks ... and I mean big time. Too often, I get called in to do an inspection of another engineer's code; only to find that the process is reminiscent of the Spanish Inquisition for the poor programmer.
src="http://i111.photobucket.com/albums/n138/krwada/Historical/torture/WheelofJustice.jpg" border="0" alt="Photobucket">
.
.
I have found that the better method is to have one engineer 'help' with the unit test of the critical code sections and to offer suggestions for improvements in the architecture / implementation.
Sign in to Reply
Lundin
6/17/2010 3:52 AM EDT
The real problem in those companies you describe seems to be that non-engineers were made engineering managers. That's a problem higher up in the company and has nothing to do with code reviews...
If the manager is needed to mediate between arguing programmers, the manager should attend. Otherwise, they have hopefully better things to do than micro-managing every code line written.
Sign in to Reply
Phil K
6/20/2010 5:55 PM EDT
Jack is right -- peer reviews are the most cost effective thing you can do to improve the quality of your code.
I'd add an additional factor beyond having a Champion (Genghis Khan's official title). You need capable, and preferably trained, facilitators to run the reviews. Those soft people skills matter a lot when egos are on the line. I've seen environments where reviews work great .. and those where they haven't. It's all about the dynamics during the meeting.
If you're worried about Manglement giving you a hard time, it can help to have scrubbed summaries of every meeting in terms of metrics. Management has a legitimate need to ensure that reviews are adding value, and keeping score in an anonymous way can help that if done thoughtfully. But that's not the same as turning in your buddies.
Phil Koopman, author of _Better Embedded System Software_
Sign in to Reply