Contents |
|
Because code (and design) review is one of my favorite soapbox subjects, I have encountered people with various amounts of exposure to the review process and to the benefits reaped from it. But regardless of their exposure, one question that invariably gets asked is: “How much time does it take?”
That’s not a bad question, its just not the right question. But let me answer it anyway.
For a first review on 2,000 lines of code, with say three reviewers, each reviewer may spend an hour, maybe two depending on the topic and the quality of the accompanying design documentation. The first review meeting might last 30 minutes, maybe longer, most likely shorter as your team gets practiced and develops a team programming style. Successive reviews might take 10 minutes on 2,500 lines of code for each reviewer and perhaps 5 minutes in a meeting. Again, with practice, a team gets really quick at this task. And the task gets quicker as fewer defects are introduced by the programmer as they invariably produce cleaner initial code as a result of the review process.
Note how small these numbers are small compared to the amount of time it takes to reproduce a defect, the time it takes to fix it, and time it takes to verify that it has been successfully removed without introducing yet more defects. I mean, how much time do you spend looking for just one bug?
Its worth pointing out that it wouldn’t be surprising to find 30 potential defects in the initial review of 500 lines of code. That kind of defect count in a newly instituted review process wouldn’t surprise me at all. Wrong types, out-of-range numbers, off-by-one errors, divide by zero cases, missing “else” clauses, double-equals “==” typos, mismatched actual and formal parameters, unreleased memory blocks, array and pointer arithmetic errors are examples we’re all familiar with. But until you have someone else look at “your” code, you’ll never know just how many defects you make. It is a universal eye-opener!
And then there are the higher level of defects a reviewer must be concerned with. Get just one of these defects in the initial review and the time-savings for all subsequent reviews on this software unit is paid for many times over -- not to mention all the debugging effort that is avoided.
My Favorite, Yet Unasked Question
So, my favorite, unasked question is:
How much time does it save?
No one ever asks this question when the code review soapbox comes out. “How much time does it take?” Who cares? “How much time does it save?” That’s what matters!!!
Well, how many bugs did you make last week? A better question is how many bugs did you hunt down last week? How long did it take you to fix them? How long did it take for you to re-test the software? How long did it take for an artist to convince you there was a problem in the first place?
If sitting in a 15 minute review would have prevented all of last week’s bugs, would you rather have had the meeting? How much time can you save your team by simple, straight-forward inspection? Again, its just eye-opening. The time saved by doing code review is far greater than the effort it takes to hunt down and repair bugs -- even for dull bank statement software.
Ah, still, how much time does it save? Well, you never have to throw a piece of software away because a programmer quit. Is that enough time saved?
How re-usable is software that has been reviewed by several people? Far more re-usable than software written in a closest by an AI programmer who didn’t understand “real-time constraints”. How re-usable is the programming team? That’s really a better gauge on how re-usable a piece of software is -- is the programmer who wrote the software still in the house? Five hands should go up! Each one an owner.
One last set of good questions posed to me at this particular Sputnik gathering included: In using code reviews and style standards and idioms, how does a new team member get assimilated into the team? Do they have to assimilate, like become part of the Borg? Do new team members reject the process or find it creatively restrictive?
In a lot of cases, I found that new team members welcomed an environment of constant peer review, it was refreshing and invigorating. The involvement and caring that a review process produces is really tremendous. But the team does have to train and educate any new team member. Assign them a mentor.
Do new team members have to become part of the collective? Absolutely. A team cannot have any outcasts or the process gets skewed. It must be applied evenly. And yes, some people do not assimilate. Let them go.
Is the review process restrictive on the creative aspects of software development? No. The point of the review process is to instill quality and reduce defects. Each new member will bring new idioms and new skills into the team. The team and Software Manager must suck that out of them and will, themselves, have to work to stay open to new ideas, fresh approaches, and better ways. Review the review process constantly, the introduction of a new team member is the ideal opportunity.
The creative aspects of software are not deterred but are actually enhanced by the sharing of one’s labor with an interested audience. It’s also better to be creative in one’s solution to a problem and avoid being creative in the implementation. Keep the implementation mundane, simple, easy to understand and easy to maintain. If keeping the code simple attenuates your creativity, then maybe software ain’t your gig to begin with.
Just as a note, our group at this one particular job was so successful, programmers and managers were breaking down the doors trying to get into our group. We were setting the standard and our code and design review processes were to thank.
My Favorite Adage Of Code Review
My favorite adage for code review is:
I thank my friend who finds fault with me.
I think this is an old Japanese saying, but if its not, I’m sorry. Regardless though, this simple adage really sums up the attitude and intent of code review. For every mistake you make in your software, you will be thanking your fellow programmers a hundred times for finding them and for teaching you how not to make the same mistakes in the future. I mean, I definitely prefer my mates finding my mistakes than I do my customers.
Code Review Can Be Fun
So how can code reviews be fun? Well, after a time, any programming group gets to know each other and everyone’s little foibles. Now imagine that you get to talk about these little foibles in a meeting every week or two. It doesn’t take long to see patterns. Some programmers make the same mistake time and time again, no matter how hard they try not to. So it’s hard not to start making fun of that.
I have seen groups get so close, so friendly, and so respectful of each other that on the day of a code review, if a reviewer noted a particular defect that a programmer constantly makes, they’d drop a cookie on the programmer’s desk early in the morning. It was always painful to get welcomed by a box of cookies first thing in the morning, but it always made you’d laugh. The implementor provides the snacks!
The point is, the code review process is extremely helpful. It can be tedious at times but yet it is so important for repeatable success and the benefits are many. Keep the process light, keep the meetings short and to the point. Avoid turning the process into drudgery. We found the review meeting themselves to be a friendly and competitive atmosphere where folks could finally, after years of writing software, they could finally show off their artistry to the people it matters most -- and to those who really get the craft. It is an awesome forum.
Do good work.
References
Demarco, Tom and Lister, Timothy, Peopleware: Productive Projects and Teams, 2nd ed., Dorset House Publishing, 1999.
Humphrey, Watts S., Introduction to the Personal Software Process, Addison-Wesley, 1997.
Maguire, Steve Writing Solid Code, Microsoft Press, 1993.
Maguire, Steve, Debugging the Development Process, Microsoft Press, 1994.
Hetzel, Bill, The Complete Guide to Software Testing, 2nd ed., John Wily & Sons, Inc., 1988.
Jacobson, Ivar et al, Object-Oriented Software Engineering: A Use Case Driven Approach, Addison-Wesley, 1992.
John Stenersen has been into 3D graphics since before most of the industry was even born (he's not that old, he just started young). Besides working on Air Force flight simulators, landing systems, shareware, and 3D games, he's been into white-water rafting, flying ultra-lights, and Scandinavian dancing. He is currently doing freelance developing through his company DarkViking Software and can be reached at mailto:DarkViking@attglobal.net For more, check out his website at http://pws.prserv.net/darkviking.