A Case for Code Review

Author: by John Stenerson

I had the occasion a few months ago to climb up onto one of my favorite soapboxes and do a little preaching. The topic centered around code reviews. I was not surprised to find that few had ever heard of this religion but were filled with questions, wonderment, and most anxious to hear the word and see revelations unfold before them.

Not to worry, this is not Dianetics for the software guru but rather one small development technique, a process that can have immense impact on the quality of software you produce as well as offer broad, positive, and measurable effects for an entire development team -- and across numerous products. So this article is aimed not just at programmers but at their managers and their manager’s managers.

The Code Review Soapbox

I attend a game developer’s user group known as Sputnik which meets once a month in Seattle. It is a casual and friendly gathering of game making enthusiasts composed of professionals and beginners, programmers, artists, sound junkies, and the occasional suit. The forum is relaxed and opened to spontaneous tangents and hence it generally covers any and all aspects of the game industry (at least once a meeting!). It’s a great learning experience with a tight finger on the pulse of game development.

This one particular evening we were sharing the secrets and ills and complaints and joys of past or present projects -- thus we were requested to and happily volunteering to air dirty laundry in public. It was great fun and quite educational. But what struck me, and it often does, is that this is an incredibly young industry rot with inexperience. It seems void or somewhat slow to adopt practices that prove nearly essential in other closely related industries.

How many post-mortems have we read touting the god-send of RCS (or Source Safe) like it’s the newest thing? How many times has the Design Document been referred to as the golden book of guidance and everyone should have one? How often do we hear that a Bug Tracking Database was essential and something we should all try? Ever heard of MS Project, Gantt charts, the critical path, or the black art of scheduling? How about meta-meetings or meta-designs? Code reviews? Anyone, anyone? Bueller?

Okay, I don’t mean to be insulting but I am plainly one of those that say the game industry needs to grow up a little, at least in its processes and methodologies. These are all staples used throughout the software world.And yet, here I sat amongst some of the best game developers in the world and though many seem to comprehend the gains and benefits of employing design and code reviews, it came across that virtually none had ever experienced these processes first hand.

The Questions

During the course of sharing secrets and dirty laundry of past and present projects, many real problems were discussed. Common things like what do you do when you lose a programmer or the lead programmer? Do you just throw their code away? Is a team better off tossing it or trying to decipher megabytes of source code? How do you integrate a replacement team member? What can a company do to prepare for (what mistakenly seems to be) the natural migration of game programmers? How can we stop writing and re-writing the same piece of technology? How do we thwart the programmer egos that bury projects? How do we train beginners and hackers? Is there hope that a software team can actually survive two projects? How can I, as a programmer, grow?

The Answer

Code review.

Actually, let me broaden the answer somewhat. Any of the discussion about code review can easily be applied to the design review process, they really work hand in hand. In fact, its difficult for me to conceive one without the other.

Software development, ideally realized, is a cyclic process of refinement and risk reduction. Most projects tend to revisit the design phase during code implementation for various reasons (I view this as actually a good thing). With the reality of milestones, deliverables, demos, and skittish management, as well as lack of complete insight and all encompassing fore-thought, it is the nature in most software projects to have design and implementation occurring in tandem (not to forget occasional visits to analysis-land and bouts of incremental testing).

But for our purposes, I’ll focus on what we seem to love most, and that’s code.

The Question Expanded

So what happens when a programmer leaves a project? Why is it so painful? And is code review really the answer?

Yes! It dices, it slices, its gets the stain out, comes in various scents and flavors, and it even does Windows. Code review is just the product you’ve been looking for! So pick up a box today and let your programmer exits not delay!

So why is it so painful when a programmer leaves a project, especially one that is a major contributor? Consider the typical game developer modus operandi: design a little, avoid meetings, do good work, do it individually, and don’t waste time. An obvious recipe for success, that is, if you’re looking for really stressful days if even one individual leaves your team.

What can a code review process do to help? First off, the process breaks down barriers and encourages sharing. In fact, that’s the essence of code review: open the box, share the knowledge, gain from the experience. No more code ownership. That’s a key element and it deserves more explanation below. But, whoa shit! No more code ownership!! I just lost half the Gamasutra.com readership, didn’t I?

The Answer Expanded

What do you mean show people my code? What do you mean no more code ownership? How am I going to know to who blame for all these bugs?

That’s a scary concept for a lot of programmers, the one about showing people your software. I mean, its your software. You probably never had anyone really look at your code except for a few teachers in high school or college. And I mean really look at your code. When was the last time you sat in a group of people who were critiquing your work, right in front of you, down to the most minute detail, and you were happy and thankful about it? If you work in the game industry, you probably have never heard of that and think its totally absurd.

Ah, but think about it for a second. If all these people are looking at, inspecting, and digesting my code, then when it doesn’t work (or has a few minor deficiencies, a-hem), who do we get to blame? Well crap, it’s not just my fault, by myself, this time. It’s everyone’s fault -- at least everyone that reviewed and approved the code. Maybe there is something to giving up code ownership.

Software development really isn’t about whom to blame when things don’t work. But its one concrete example that actually gets people to consider giving up sole ownership of code. Whatever works.

Exposure

The idea behind a good code review process is that the software is exposed. It is examined at various levels, dissected for various objectives, and scrutinized for all manner of quality (or lack of). The purpose is to recognize defects through inspection. The process is designed to bring the cumulative skill and experience of the entire programming team (over time) to bare on any and every piece of the software. It is the means to harness the strength of the collective and wash away the weaknesses of the individual. A little Trekkie philosophy can go a long way. The Borg have it figured out.

The code review forum also provides an ideal learning environment. Where do programmers today go to learn new tricks, refine their methods, test their prowess, and grow their skill? Primarily its through individual effort, i.e. time-consuming (even career-consuming) trial and error software exploits. Programmers constantly re-write the same technology so they can learn from the experience. Through code review, experience can be passed on, much faster and with much greater variety. And it’s a repetitive process, one that re-enforces good programming practices, one that allows you to teach as well as learn, and it can actually be fun! Yes, its true, code review, a meeting (for god’s sake), can actually be fun. I’ve seen it, I’ve lived it, it really works.

The Mechanics

I was not terribly amazed when I was asked (and later confronted to elaborate) on how to do a code review. As I alluded to earlier, this is a well-understood process that no one has done before. It took a lot of brave effort, in the friendly gathering of Sputnik, for someone to actually admit they didn’t really know how code reviews work.

Cool, so let’s talk about the mechanics of code review.

I have worked in a variety of software industries ranging from games to military flight simulators to aircraft navigation systems and to boring stuff like bank statement report generators and configuration management tools. And each experience where code review was part of the development life-cycle, there was always success, of varying degrees to be sure, but each project reaped benefits. And notice, I did say “success” as each job gained through use of code review and none suffered. More importantly, however, each team member gained in skill, in capacity, in proficiency, in creativity, in interactive skills, and in modesty.

For the description of the mechanics, I’m going to draw on my most successful work experience where a large group of young engineers incorporated code (and design) review as part of the daily diet. Yep, that’s daily. Imagine a process where you meet (eek!) nearly everyday and it saves time!

Here’s how it works in summary: someone (in our case often two someones) writes a small pile of software according to some documented set of requirements (the output of a previous design review), compiles it and performs some initial testing, then prints out onto paper (this is very important) copies of the source code for each reviewer and gives the reviewers a day or two to review it. At a scheduled meeting, a small group of multi-talented individuals gather and point out problems, list things to be changed, ask and answer questions, and ultimately (by iteration of code-test-and-review) the implementation is approved according to a well-defined set of criteria. Sounds simple enough. Its repetitive and it gets very mechanical (and that’s a good thing).

The Players

The Implementor: This is the individual who’s software is being reviewed. I’ll belay the obvious code ownership question for the moment.

The Reviewer(s): Typically this is more than one person, the number varying depending on the software’s content and its overall importance. The number of reviewers also varies depending on how many iterations a piece of code has been through, generally the number drops as the software matures. Reviewers are programmers, designers, managers, senior and junior people alike.

The Software Manager: In game parlance, that would be either the Lead Programmer or the Technical Director whose responsibility it is to track and schedule code development and the review process. Note that the software manager is not the sole reviewer though they tend to be involved in most reviews.

The Materials

The Source Code: This is the source code, batch files, grammars, external references (books, technical papers), test cases, anything relevant to the application’s functioning, creation, or validation. And it needs to be distributed on paper. Yes, kill a few trees, save your project (that’s from a tree-loving Oregonian who knows a renewable resource when he sees one).

A Red Pen: Each reviewer must use a red pen to mark corrections on paper. Legible, pertinent comments are the goal since a meeting (ha!) is not always required for an effective code review, but good ink is.

The Design Requirements: More properly categorized as an approval criteria, design documents that specify requirements is essential for the implementor and the reviewers. This is what defines what the implementor is to (or should have) created. Without the goal well defined for each participant, there will be no consensus on whether a piece of software does all of what it needs to do.

The Approval Criteria: This consists of coding standards, list of abbreviations, documented coding idioms, validation tests, and possibly a Software Quality Assurance Manual. As I’ll point out below, how pedantic you want to be is up to each group, but regardless, there must be a well understood set of criteria by which the reviewers can test the software against.

When To Schedule A Code Review

Anyone can schedule a code review. Generally its at the behest of the Software Manager. But when is that?

Typically, we found that 500, one thousand, or as much as two thousand lines of new code marks a good time for a review. The initial review needs to be meaty enough to represent all the complexities of the software component under development. At the same time, the team needs to be careful that the approach used in the code is scrutinized before it becomes too large and difficult to change. This is an iterative process, so take it in small bites. How much code to review during each iteration will vary by group, by subject, and the skill level of the implementor.

It was common during my most successful project for each programmer to have some subset of their software under review at least once a week. Two weeks between reviews wasn’t uncommon for any particular software unit. But at three weeks, the implementor themselves would be screaming for a review. Getting good, timely feedback is essential to the entire process and coding in isolation for too long is dangerous.

Another scheduling consideration is the completion of functionality. As pieces come on line, don’t delay their review. Verify and approve software as being complete as often as you can -- everyone gets a boost from small successes. We found, also, that people tended to stall out and fail to move onto the next task until they were reviewed and approved.

If an individual seems to have a particularly sticky algorithm, can’t solve a problem, can’t decide an implementation approach, or has a history of defect creation -- schedule them more often. The review can be used to solve problems as well as to identify them.

As a software unit matures, meaning that its functionality is nearing completion, test suites are firming up, and the software has been reviewed a number of times, we found that reviews occurred more often with the benefit that the review time dropped dramatically. Also, with each iteration, the number of reviewers necessary to get good coverage declined. A final review might consist of the implementor and the Software Manager putting a checkmark on a schedule sheet.

Who Should Participate In The Review

Certainly the implementor and often the Lead Programmer (a.k.a. Software Manager) should participate in the review, but also a selection of other programmers. Seldom does any piece of software require the entire programming team. It depends on the scope and importance of the unit being created.

The Software Manager should attempt to get enough skill and experience coverage to recognize and discover software defects. He also is trying to schedule junior programmers as reviewers so they learn during the process. Senior and skilled individuals are encouraged to expand their knowledge and to pick up new methods and to teach.

The designer or author of the requirements is an excellent participant in the earliest stages of review as the initial implementation often ferrets out design flaws or deficiencies. It is not uncommon that a design is rejected and sent back for refinement in the first review.

One of our underlying goals in scheduling review participants was to provide insurance, not just in software quality, but in the case where a programmer leaves the team. Hence, we often assigned a software unit to a primary implementor and a second programmer. The primary was responsible for the software and for partitioning the unit so two people could write components of the same unit. Thus with constant review, there is no piece of software that only one person understands. If not a second programmer, then we would assign a primary reviewer who would participate in the unit’s development until its completion. Thus the second programmer or the primary reviewer could pick up where the dearly departed programmer left off and with little delay.

No piece of software should be comprehended by only one person. There is no implementation mystery big enough that it cannot be described and comprehended by at least two people within an effective review process. Lights should start flashing red if the phrase “only Guru A knows how that works” is uttered during the project.

The Code Review Meeting

Schedule the code review meeting for one or two days after handing out the source code to each reviewer. Make sure each reviewer has a recent copy of the design requirements of the software unit being reviewed. They will need ample time to comprehend and inspect the work. And, as always, start the meeting on time.

Our meetings were always driven by one of the reviewers, the primary reviewer, and not the implementor. The primary reviewer would then be tasked with keeping the review focused, making sure that all issues were sufficiently covered, that each reviewer and the implementor had opportunity to state opinions, facts, observations, what-have-you. The primary reviewer was charged with the authority to resolve any disputes, should there be any.

The meeting would start with the primary reviewer asking the implementor if there were anything they wanted to point out or highlight about the unit under review. Then there was usually a request for general comments by any of the reviewers. Often times there would be a change that would need to be made throughout the unit, so it would get dealt with up front and only one time.

Eventually, the review participants would go through the software, page by page, routine by routine, and note any deficiencies or defects or concerns. The implementor would, on a clean copy of the source code, mark down any changes or comments in red. This was the master copy of the reviewed unit which served as the guide for any repairs.

Often times only the first pass required a routine by routine inspection. In later iterations, the master copy of the source code would be used as a guide to check off and verify that all changes were successfully completed by the implementor. Verifying (and just recalling) the changes is extremely important. For each meeting, a new master copy listing required changes would be generated.

We seldom took meeting minutes and let the master copy serve that purpose. Be sure to date the master copy and note who the reviewers were. We kept the master copies in software folders (an Uncle Sam requirement) and used them as reference. All the reviewer’s copies were shredded, but the recycle bin is probably just as good. Disposing of out-of-date material is simple but extremely important house cleaning.

If there were action items as a result of a review, either the implementor or the primary reviewer would forward them to the Software Manager. These might include changes needed to the Design Document, changes needed to other software units, or just about anything.

The meeting would typically conclude with the implementor suggesting how long it would take to make changes, whether new components would be considered in the next review, and whether any further reviews were necessary at all.

Mold The Method to the Talent

During my soapbox moments, I have been asked if junior programmers should review code of a senior programmer. Yes. And the opposite? Yes. Should managers be involved? Yes. Besides being focused on bug hunting, the review process is also a concentrated learning and teaching session which has longer lasting affects than a seminar or classroom course. Managers and senior people need training and re-enforcement as well, so open the box and expose the software. Egos will fade, modesty will reign, and everyone will grow and benefit.

Avoid having the same people review the same person. Mix it up, use a round robin process, and crossbreed the skill sets.

As time goes by, the strengths and weaknesses of the reviewer (and the programmer) will become apparent. Some reviewers are good at picking on identifier names, others on initial case problems, missing tests cases, missing or misspelled words in comment blocks, what-have-you. Recognize these talents and blind spots and employ them or address them by crafty scheduling.

So how picky do reviews get? You might have noticed I mentioned “identifier names” and misspelled comments. That sounds scary. This sounds like a lot of work. How could this possibly save time? Egads!

That’s where it becomes quite obvious that each development team or company must decide what is important to scrutinize in the review. It is utterly necessary that each programming team grow and apply their own standards. If you don’t care about function names, don’t. If you don’t care about copyright headers in source files, then don’t. If you do care about a “default” clause for every “switch” statement, then make that part of your coding standard. The point is that as a group, you define your own expectations, make them clear, document them, and then live up to them. As a group you can change and alter your level of quality and how much you can ask of your team. Tailor the expectations to the talent you have and conscientiously decide what your approval criteria will consist of.

My recommendation (and druthers) is that everything in the source code is open to scrutiny. If you can’t justify a 3,000 line function, its name, the “goto” you used, then those problems need to be fixed. If your reviewers can’t understand an algorithm, that’s a good indicator a different approach is needed, even if it seems to work.

One of our touchstone gauges was “Will a maintenance programmer, 5 years from now, someone you will never meet, be able to fix or enhance this module based on the source code and design documents?” If not, the source code is “broken” and needs to be inked in red and reviewed again during the next iteration.

Another utterly important aspect of code review is the responsibility reviewers have toward the process. It requires commitment but it also requires flexibility. The intent is to prevent defect creation and to recognize them, not to set up road blocks.

Another adage we followed was “If you can’t offer a better alternative, then don’t offer one.” In other words, be selective in what you can’t live with -- just because you don’t like the approach used doesn’t mean its not valid. Though you are responsible as a reviewer to note problems, if the software functions sufficiently, that’s good enough. Don’t waste time making every piece of software perfect. We’re writing games, not landing airplanes.

Here’s another healthy realization: Keep in mind that with five programmers, for example, there will be five or more excellent solutions to any given problem, none of them alike. In this case, 80% of the software in your game will not be written the way you would do it. Lose 4 out of 5 arguments and have faith in your fellow programmer’s ability to meet the design requirements. The code review is meant to guarentee successful completion of the coding task, not to define or write the code, that’s the responsibility (and authority) of the implementor.

With time, every group I’ve been involved with develops their own methods and standards of quality. This happens quickly. Each group also eventually tunes itself. After reviewing Programmer B’s work for 6 months, you’ll understand their coding method quite naturally and your comprehension of their work will increase and with reduced effort.

After more time, a group tends to narrow in on a “team method” or style of implementation. In other words, a solution written by one programmer will use the same style and structure any other programmer of the team would have used -- even down to using the same identifier names. It is a bit eerie when the team consistently uses the same names for similar purposes in different software units, but that is a mark of success when members of the collective use similar schemes. The defect count will drop dramatically.

Code Ownership: A New Meaning

That brings me back to the code ownership question.

Who owns the code? Is it the person who wrote the first rendition? Did the second programmer not write part of the unit? Did the reviewers not approve the implementation? Could the primary reviewer complete a module? Is there any reason a unit couldn’t be started by one individual and completed by another? Don’t we re-use software anyway?

As I have noted over several projects, the code review process intentionally blurs the code ownership issue. In the best case, we had a team that couldn’t easily identify who had written which lines of code just by looking at them. (Is that true in your team?) It could have been any member of the team. That is a strength and mark of success. This particular team had refined its techniques to incorporate all the skills of the team as a whole and replaced variety and confusion to an easily comprehended, consistent style.

I guess I would redefine code ownership this way: every piece of code is owned and comprehended by several people. Beyond that, who cares?

Ah, but there is a good reason to note who the owners are and to note that the “owner” changes over time. The initial implementor is the person most likely to fix a problem the fastest. That’s kind of important to track. So the initiator does tend to “own” the software for a long time. However, they also happen to be the least likely person to recognize a defect in the software (or they wouldn’t have put the defect there in the first place, right?).

Any of the reviewers should make excellent maintenance programmers for units they reviewed, at least for the duration of the defect they are repairing. Hence the reviewers become owners of the software for a time and subsequently they become the best person to fix their own repairs.

Another common practice is for junior programmers to extend the functionality of an existing unit. So they also become owners, too, as do the reviewers of the enhancement work.

See how the ownership gets rather blurred? There are many owners over time. So that also means that the responsibility for successful, continued functioning of a software unit gets shared over time. Not one person is solely responsible for any one piece of code over the life of that unit.

Thus when a single programmer departs for another competing game company, there will always be another “owner” capable of insuring the “abandoned” software remains current and valid for the project. Nothing ever has to be thrown away because the software has been reviewed, approved, and maintained up to the established project standards. The programmer departed, not the software.

Meta-This And Meta-That

What is this “meta-“ thing and why is it so important?

In the case of a meta-design, this is the description of what a design should look like. It details what is sufficient in a design document such that a programmer can build the desired piece of software. A meta-design can be a set of boilerplates.

A meta-meeting is the meeting to decide how meetings are run. Will there be a meeting notice? Who needs to attend? Is there a time-limit? Is an agenda required? Is a note taker part of every meeting? Who steers the meetings, ends arguments, and how? What about publishing meeting minutes?

The point here is that it is essential to have a meta-review. In other words, do not under-estimate the importance of deciding exactly how your group wants to perform reviews, the entire process, over the entire project. You must train and educate everyone in the process. No one can be exempt. Review the review process and refine it.

This also includes defining exactly how nit-picky reviews need to be on a per project basis. This of course means that a group must define the level of software quality they wish to attain. It takes a concerted effort to establish the ground rules. Be sure to include a method to refine the rules.

Idioms & Consistency

Here are two helpful hints to apply to any software project. The first is to employ idioms. In other words, attempt to reduce the coding style and techniques used to a small number of well understood idioms. My latest project used doubly-linked lists for everything and instead of using a “for loop” to iterate through the list, we used a “while loop” comparing pointers for the termination condition. Its just the method we used religiously.

If the team employs idioms consistently, the form becomes recognizable without effort. It also reduces the maintenance issues considerably. Avoid doing surprising things in your software and use copious comments if you have to deviate from the norm. Use of idioms can reduce the comment overhead.

And then, secondly, there’s consistency. Having been employed to develop coding standards and style guides, I can say that consistency is the overall rule of thumb to apply throughout any software project. If you spell a word incorrectly (like “Macintosh”), spell it wrong everywhere. If you employ a flawed idiom, employ it consistently or fix it everywhere. If you use an open brace “{“ with every “if” statement, your teammates should be doing the same thing as should your maintenance programmers.

So when it comes to the code review process, be consistent in applying approval criteria. There should be no mysteries and meeting the criteria should be easy. Be consistent in applying standards, the same standards, to every piece of software and to each individual. Make no exemptions, they’re hard to remember and seldom get documented.

My Least Favorite Question

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 [email protected]. For more, check out his website at http://pws.prserv.net/darkviking.