Last month I wrote an introduction to the concept of code inspections, a technique that can hugely accelerate schedules by eliminating bugs before they percolate into testing. I drew an analogy to the quality movement that revolutionized manufacturing. Others made a similar comparison, notably Mary and Tom Poppendieck in their recommended book, Lean Software Development .1 In manufacturing one strives to eliminate waste, which, for firmware, includes bugs injected into the code.
Code inspections are nothing new; they were first formally described by Michael Fagan in his seminal 1976 paper “Design and Code Inspections to Reduce Errors in Program Development.”2 But even in 1976 engineers had long employed design reviews to find errors before investing serious money in building a PCB, so the idea of having other developers check one's work before “building” or testing it was hardly novel.
Fagan's approach is in common use, though many groups tune it for their particular needs. I think it's a bit too “heavy” for most of us, though those working on safety-critical devices often use it pretty much unmodified. I'll describe a practical approach used by quite a few embedded developers.
First, the objective of an inspection is to find bugs. Comments like “man, this code sucks!” are really attacks on the author and are inappropriate. Similarly, metrics one collects are not to be used to evaluate developers.
Secondly, all new code gets inspected. There are no exceptions. Teams that exclude certain types of routines because either they're hard (“no one understands DMA here”) or because only one person really understands what's going on will find excuses to skip inspections on other sorts of code, and will eventually give up inspections altogether. If only Joe understands the domain or the operation of a particular function, the process of inspection spreads that wisdom and lessens risk if Joe were to get sick or quit.
We only inspect new code because there just isn't time to pour over a million lines of stuff inherited in an acquisition.
In an ideal world an inspection team has four members plus maybe a newbie or two that attend just to learn how the products work. In reality it might be hard to find four people so fewer folks will participate. But there are four roles, all of which must be filled, even if it means one person serves in two capacities:
• A moderator runs the inspection process. He finds a conference room, distributes listings to the participants, and runs the meeting. That person is one of us, not a manager, and smart teams insure everyone takes turns being moderator so one person isn't viewed as the perennial bad guy.
• A reader looks at the code and translates it into an English-language description of the statement's intent. The reader doesn't say: “if (tank_press>max _press)dump(); ” After all, the program is nothing more than a translation of an English-language spec into computerese. The reader converts the C back into English, in this case saying something like: “Here we check the tank pressure to see if it exceeds the maximum allowed, which is a constant defined in max_limits.h . If it's too high we call dump, which releases the safety valve to drop the pressure to a safe value.” Everyone else is reading along to see if they agree with the translation, and to see if this is indeed what the code should do.
• The author is present to provide insight into his intentions when those are not clear (which is a sign there's a problem with either the code or the documentation). If a people shortage means you've doubled up roles, the author may not also be the reader. In writing prose it has long been known that editing your own work is fraught with risk: you see what you thought you wrote, not what's on the paper. The same is true for code.
• A recorder logs the problems found.
During the meeting we don't invent fixes for problems. The author is a smart person we respect who will come up with solutions. Why tie up all of these expensive people in endless debates about the best fix?
We do look for testability issues. If a function looks difficult to test then either it, or the test, needs to be rethought.
Are all of the participants familiar enough with the code to do a thorough inspection? Yes, since before the meeting each inspects the code, alone, in the privacy of his or her own office, making notes as necessary. Sometimes it's awfully hard to get people other than the author to take the preparation phase seriously, so log each inspector's name in the function's header block. In other words, we all own this code, and each of us stamped our imprimatur on it.
The inspection takes place after the code compiles cleanly. Let the tools find the easy problems. But do the inspection before any debugging or testing takes place. Of course everyone wants to do a bit of testing first just to ensure our colleagues aren't pointing out stupid mistakes. But since inspections are some 20 times cheaper than test, it's just a waste of company resources to test first. Writing code is a business endeavor; we're not being paid to have fun–though I sure hope we do–and such inefficient use of our time is dumb.
Besides, in my experience when test precedes the inspection the author waltzes into the meeting and tosses off a cheery “but it works!” Few busy people can resist the temptation to accept “works”, whatever that means, and the inspection gets dropped.
My data shows that an ideal inspection rate is about 150 lines of code per hour. Others suggest slightly different numbers, but it's clear that working above a few hundred lines of code per hour is too fast to find most errors. 150 LOC/hr is an interesting number: if you've followed the rules and kept functions to no more than 50 LOC or so, you'll be inspecting perhaps a half-dozen functions per hour. That's 2.5 LOC/minute, slow enough to really put some thought into the issues. The cost is around $2/LOC, which is in the noise compared to the usual $20 to $40/LOC cost of most firmware. Of course, the truth is that inspections save time so have a negative cost.
It's impossible to inspect large quantities of code. After an hour or two one's brain turns to pudding. Ideally, limit inspections to an hour a day.
The best teams measure the effectiveness of their inspection process, and, in fact, measure how well they nab bugs pre-shipping in every phase of the development project. Two numbers are important: the defect potential, which is the total number of bugs found in the project between starting development and 90 days after shipping to customers, and the defect removal efficiency. The latter is a fancy phrase that is simply the percentage of bugs found pre-shipping.
Clearly you can't measure defect potential unless you track every bug found during development, whether discovered in an inspection or when Joe is furiously debugging his code. Being just a count it's painless to track.
If your inspections aren't finding at least 60% of all of the bugs there's something wrong with the process. Tune as needed. 70% is excellent, and some companies achieve 80%.
(To put more numbers around the process, Capers Jones showed in his paper “Measuring Defect Potentials and Defect Removal Efficiency” (Crosstalk magazine) that the industry averages an 85% defect removal efficiency.3 At the usual 5 bugs injected per 100 LOC that means shipping with 7.5 bugs per KLOC. Achieve 60% efficiency just in inspections and figure on 4.5 per KLOC. Jones found that the lowest development costs occur at a defect removal efficiency of 95%, or 2.5 bugs/KLOC. According to a vast library of data he sent me, projects run at 95% cost nearly 50% less to develop than ones idling along at 85% yet they ship with three times fewer bugs. And he calls an efficiency of 75% “professional malpractice.”)
Inspections are irrevocably tied to the use of firmware standards. If a team member complains about a stylistic issue, the moderator says: “does this conform to the standard?” If the answer is “yes,” the discussion is immediately closed. If no, the proper response is a simple: “fix it.” No debate, no expensive discussion. Don't use inspections and the team will surely drift away from the firmware standard. The two processes are truly synergistic
Managers have an important role: stay away. Enable the process, provide support, demand compliance, but never use the inspections as a way to evaluate people. This is all about the code, and nothing else.
Managers must always ensure the team uses inspections and all of the other processes even when the delivery date looms near. Can you imagine getting on a plane, walking in the door, only to overhear the pilot say to the co-pilot: “We're running late today; let's skip the pre-takeoff checklist.” What amateurs! Management can't let the threat of a schedule dismember proper processes… which, after all, shorten delivery times.
Inspections inspire plenty of debate, but few who have looked at the data doubt their importance. In today's agile world some advocate a less formal approach, though all buy into the many-brains-is-better-than-one theory. For an example of lighter-weight reviews, see www.methodsandtools.com/archive/archive.php?id=66.4
The agile community buys into inspections. As Kent Beck wrote in eXtreme Programming Explained : “When I first articulated XP, I had the mental image of knobs on a control board.5 Each knob was a practice that from experience I knew worked well. I would turn all the knobs up to 10 and see what happened. I was a little surprised to find that the whole package of practices was stable, predictable, and flexible.” One of those knobs is code inspections, embodied in XP's pair programming.
The most complete work on inspections is Software Inspection , by Tom Gilb and Dorothy Graham.6 It's a great cure for insomnia. Or consider Peer Reviews in Software , by Karl E. Wiegers.7 Written in an engaging style, rather like the Microsoft Press books, it's an accessible introduction to all forms of inspections, covering more than the traditional Fagan versions. One of my favorite books about inspections is a free one available from SmartBear Software called Best Kept Secrets of Peer Code Review . 8
Then there's Software Inspection: An Industry Best Practice by Bill Brykczynski (Editor), Reginald N., Jr. Meeson (Editor), David A. Wheeler (Editor).9 Unhappily it's out of print but used copies are usually available on Amazon. The book is a collection of papers about successful and failed inspections. But it, but don't read it. When I can't stand the thought of yet another inspection I pull it off the shelf and read one paper at random. Like a tent revival meeting it helps me get back on the straight and narrow path.
Inspections yield better code for less money. You'd think anything offering that promise would be a natural part of everyone's development strategy. But few embedded groups use any sort of disciplined inspection with regularity. The usual approach is to crank some code, get it to compile (turning off those annoying warning messages) and load the debugger. Is it any surprise projects run late and get delivered loaded with bugs?
But consider this: why do we believe (cue the choir of angels before uttering these sacred words) open-source software is so good? Because with enough eyes, all bugs are shallow.
Oddly, many of us believe this open-source mantra with religious fervor but reject it in our own work.
Jack Ganssle () is a lecturer and consultant specializing in embedded systems' development issues. For more information about Jack .
1. Poppendieck , Mary and Tom. Lean Software Development . Addison-Wesley, 2003.
2. Fagan, Michael, “Design and Code Inspections to Reduce Errors in Program Development,” IBM Systems Journal , Vol. 15, No. 3, 1976, available on-line at www.research.ibm.com/journal/sj/153/ibmsj1503C.pdf.
3. Jones, Capers. “Measuring Defect Potentials and Defect Removal Efficiency,” Crosstalk magazine, June, 2008, www.stsc.hill.af.mil/crosstalk/2008/06/0806Jones.html,
4. Cohen, Jason. “Four ways to a Practical Code Review: How to almost get kicked out of a meeting,” Smart Bear Software, www.methodsandtools.com/archive/archive.php?id=66
5. Beck, Kent. eXtreme Programming Explained: Embrace Change , Addison-Wesley Professional, 1999.
6. Gilb, Tom and Dorothy Graham. Software Inspection , Addison-Wesley, 1993, ISBN 978-0201631814.
7. Wiegers , Karl E. Peer Reviews in Software , Addison-Wesley, 2001, ISBN 978-0201734850.
8. SmartBear Software. Best Kept Secrets of Peer Code Review, available for free online at www.smartbearsoftware.com.
9. Brykczynski, Bill, Reginald N., Jr. Meeson, and David A. Wheeler (Editors). Software Inspection: An Industry Best Practice , IEEE Press, 1996, ISBN 978-0818673405.