Running an Effective Code Review

“Oh, don’t get me started on code reviews!” says Gary Heusner, client partner at custom software developer Geneca, in what can only be described as a Marvin the Android voice. “For many shops, code reviews are as prevalent as disaster recovery exercises.”

To run a successful code review, your first step is to ensure that the code review happens. The code review process typically is among the first items jettisoned from a project, Heusner sighs, “Usually right before someone trims user acceptance testing to less than a week for a four-month e-commerce project.” That can occur even in software development departments where the team personally cares about quality. Any mention of “code review” elicits comments like, “Wouldn’t it be great to do them?” or “I heard someone did one last project” or “Is it worth the effort and money to hold code reviews since QA will still have to test everything anyway?”

If you got this far, you are already sold on the benefits of code reviews. (If you need extra encouragement, see 5 Reasons For Software Developers to Do Code Reviews (Even If You Think They’re a Waste of Time.)

But it’s one thing to say you’re going to do code reviews. And it’s another thing to know how to go about the process right, so that the end result is the best, most joyful application possible. Ideally, you also build a collaborative team environment, create a more responsive development process and, oh yeah, have more fun at work. In this article (and its accompaniments), I share the wisdom gathered from dozens of passionate software developers (oh boy, are they passionate!) about when and how to do code reviews, who should sit at the table and the dire consequences when code reviews are done wrong. And incidentally, they are done wrong a lot.

I’ve tried to make this a definitive guide to code reviews, which means that I’ve split the various issues into several articles. That lets you can focus on solving the problem that’s getting your shorts twisted into a knot. But I like to think you’ll want to read the entire thing; you don’t have to do so in any particular order. Here’s everything in this package:

• Running an Effective Code Review (this article)

• 5 Reasons for Software Developers to Do Code Reviews (Even If You Think They’re a Waste of Time)

• What to Look for in a Code Review

• Making Code Review Software Tools Help, Not Hinder

• How to Lead a Code Review (let’s set up and lead the review)

• How Not to Run a Code Review

• Doing Spot-On Code Reviews with Remote Teams

What Kind of Code Review Is This?

Before you go barreling into the conference room armed with a stack of printouts and the phone number for the local pizza delivery joint, make sure that you know exactly why you’re getting the team together. Code reviews can have many purposes, and you will have a Very Bad Day if everyone has a different idea of what the purpose of this review is.

You may want to schedule different code reviews for each aspect of the project, such as one that looks at security issues and another that pays particular attention to the application’s performance.

“The first step should be to determine why you are reviewing the code,” suggests Micheal Lalande, director of technology at QLogitek, a SaaS supply chain solution provider. “This should come from your design-time discussions, where the core non-functional requirements have been made. These can include, but are not limited to, globalization, performance, security and supportability.” Re-iterating the purpose at the beginning of the meeting helps the team put its attention on items that deliver the biggest ROI, Lalande says. “For instance, if you are looking at performance, you won’t care about the procedure that is called in exception cases, so accepting the results of an automated code review will suffice.”

Picking on one thing at a time also ensures that developers dive headlong into a single aspect of the software and don’t try to do too much at once. “Too often, a poorly run code review has everyone focus on the same superficial issues,” says Theron Welch, software mentor at the Microsoft Asia Center for Hardware, who is helping to build a team in China. “A code review checklist can help encourage a smaller group to focus deeply on a specific area, another group to focus on a different area, and so on. This helps the code review achieve depth.”

The review’s goal—both this specific review and the process in general—is informed by the business’ needs, its institutional bias, the state of the team members and the role of the participants. For example, Jack Danahy, founder and CTO of Ounce Labs, says, “When you’re in a bank, you’re used to a vault mentality. Within financial services, there are two big concerns: privacy and integrity of data, and the non-reputability of that data.” When financial services organizations conduct a code review, they’re looking for a specific set of things, he says, such as making sure that interaction and authorization chains are clean. Danahy adds, “In the public sector, the purpose is much more around making sure the application worked the way they expected. There’s not as much of a detailed focus but rather looking at the general characteristic, ‘Does it work?'” Typical code reviews are about generic policy, such as making sure inputs are validated, as opposed to a more granular policy in which developers have to make sure private data is stored appropriately, he says.

It’s important to keep the team’s attention on the goal of this code review, and to avoid distraction with other issues. An extremely common mistake, says SOA specialist Mike Kavis, is for reviewers to challenge the design. “The design should have been ironed out in the design review and should not be part of the discussion in the code review,” he says. “This is why it is critical that design reviews take place; otherwise the code review process can send the developer back to the drawing board.”

Individuals’ backgrounds also color the review, points out J. Schwan, managing partner of Solstice Consulting, a Chicago-based technology management consulting firm, depending on whether the developer is junior or senior. For example, Schwan says, in a code review for junior developers, goals should include adherence to the design or architecture defined during the design phase; ensuring that the code is written to perform as efficiently as possible; and use of common code modules. “For senior developers, a peer code review is often more effective,” Schwan says. “The goal can then be more focused on ensuring utilization of common code modules and identifying other common code modules that can be reused by other parts of the system.”

Next: What’s the right time?

When to Schedule That Code Review

The trick, of course, is to run code reviews soon enough and often enough to find problems, without getting in the way of writing the code in the first place.

THE ULTIMATE GUIDE TO CODE REVIEWS

Running an Effective Code Review

5 Reasons for Software Developers to Do Code Reviews (Even If You Think They’re a Waste of Time

What to Look for in a Code Review

Making Code Review Software Tools Help, Not Hinder

How to Lead a Code Review

How Not to Run a Code Review

Doing Spot-On Code Reviews with Remote Teams

For many teams, the code review cycle should start soon after design reviews are done. Steve Porter, technical program manager at Imaginet Academy, suggests that team embrace design reviews instead of code reviews, because the latter can sometimes occur too late to correct any errors. “Design reviews help identify the various paths you can take to get to the final solution and help the developer or team make the right choice early, instead of after time has been spent writing code for review.”

Christopher Buchino, director of software engineering at GotVMail Communications considers the code review to be particularly useful at the start of a new project or when someone new joins a project. That helps developers learn the team’s standards, style-wise and architecturally. “Having uniformity in the code base is extremely helpful towards maintainability, and this is a good way to get people writing code in a similar fashion,” Buchino says.

Suggests QLogitek’s Lalande, “Do the code review as soon as you feel comfortable with the unit tests on code that have been identified as requiring manual review.”

Overall, however, experienced developers feel that code reviews ought to be a non-negotiable part of the software development lifecycle—and should not be voluntary. Alex Russell, the Dojo toolkit guru who is now at Google working on the Chrome Web browser, says, “Code reviews don’t work when they’re optional. They need to be as much a part of your routine as merging from trunk is.”

Jay Deakins, the founder and president of Deacom, an ERP software producer for the building component and batch process industries, says, “The software code review process should really be a check off point of a well planned development cycle, not a beginning step.” Yet, he points out, the code should be of reasonably high quality before it is reviewed in a formal code review.

More important than when may be how often. And that should be regularly. Russell says, “Code reviews a month after you wrote something might as well be a post-mortem. Getting feedback while your head is still ‘in the code’ is significantly more valuable than reviews of code you’ve forgotten the gory details of.”

For some developers, this means a weekly formal review. For others, it’s a short daily meeting.

Daily?! Won’t that take too much time? Not if you’re applying Agile processes, such as block diagrams before code implementation and informal scrums during implementation, according to developer Chuck Brooks.

Jason Cohen, founder of Smartbear Software (which, you should be aware, sells tools to help in the process), suggests that everyone on the development team try doing code reviews for just one week for 20 minutes per day. “Set your pessimism aside for only that week, and give it a fair shake.” Cohen urges development teams to measure the time spent and how many defects you find (just during this trial period), then compute “minutes per defect we find.” You’ll probably find it’s between 8 and 15 minutes, Cohen says. “Is there any other process at your company which can uncover and fix defects at that rate? Doesn’t that mean it’s a good idea?”

For more of the use of software tools in the code review process, see Making Code Review Software Tools Help, Not Hinder.

None of this will do any good, of course, if the participants are unwilling to be in the room.

Cop the Right Attitude

There are two real management dangers in code reviews: ego and politics. Most developers raised the subject of establishing the right attitude in creating an effective code review. This is usually in the context of ensuring that developers are willing to listen to input on how to improve the code—otherwise, why are you bothering?—and avoiding the unfortunately human tendency to turn these meetings into pissing contests. (For more on how to run the meeting, see How to Lead a Code Review.)

To a great degree, this is about trust. How much can the developer trust others she works with to give her valid and useful criticism? In a healthy environment where the culture is supportive and everyone wants to help other team members, generally this is not a problem. (Appreciate it.) But then again, it might be. There is nothing so personal as the art someone creates, and developers can be awfully protective of their work and anxious for praise. (So can article authors, by the way. I’m just sayin’.)

Doug Carrier, the product manager for Compuware Devpartner (note: another vendor), points out that developers can sometimes identify themselves a little too closely with the code they produce. “Code reviews can make the developer feel unduly criticized, humiliated or otherwise bullied by their peers or superiors. Development Alpha types typically emerge and defend their supreme role in the code review process. This can unleash a range of organizational, behavioral and emotional issues within a development group.”

Be realistic, especially if you’re team lead or manager. “While the objective of the code review process is to improve the quality and maintainability of the code, the live code review ritual is rife with elements of ego and personality that can make the process quite painful for developers who just want to do a good job,” says Carrier. (See How Not to Run a Code Review for what happens when the politics get out of control. Not that this would happen in your shop.)

Avoid any sense that the review is a punitive measure, that it is about damage control, or that it is exclusively targeted toward the developers whose work is being reviewed,” advises Jeff Benson, technical lead at Geneca. That’s especially true with journeyman-class developers. “As coders become more experienced, reviews are often seen as invasive and doubting,” says Michael Ryding, an IT solutions consultant at AXA UK.

Open-mindedness is necessary on both parts, says Smartbear’s Cohen. “If the participants want the code review process to fail, they will win. It’s easy to spend lots of time and find few bugs, if you’re intentionally not trying.”

So if you’re running a code review, or are simply a participant in one, it’s important to set your expectations with the right configuration. “The purpose of code reviews is to learn from one another; without detailed explanations or open discussions, no one wins,” says Benny Czarny, founder and CEO of OPSWAT, which makes development tools and data services for security features of endpoint applications.

Before each session, it’s a good idea to re-establish the rules (respect, humility, be constructive) and reiterate this code review’s goals (relevance of design with requirements, coding standard, locate errors), suggests senior software engineer Michael Doubez.

Make the meeting fun, lively and upbeat, and cultivate an atmosphere of shared enlightenment. Ryding UK explains, “Everyone needs to know they can all learn something and they’re not beating up and laughing at that little guy.”

Turn a potentially threatening situation into an opportunity to “geek out” and talk shop, urges Geneca’s Benson. “Keep the level of humor high. At the same time, balance this with an attention to quality and don’t compromise the integrity of the architectural vision.” The vision should be the benchmark for evaluating code, rather than appealing to subjective pronouncements about the “right” and “wrong” way to do things.”

“When the reviewer offers a suggestion, or asks a question, the author shouldn’t become offended and shutdown,” says Czarny. “Similarly, a reviewer should never demand code be changed without explaining why they believe there is an issue.

Attitude Adjustment

A Few Tips To Make Code Reviews Worthwhile

E. William Horne, systems architect at William Warren Consulting, has several bits of advice to share, all of which come down to don’t take it personally. “It’s about finding errors before they bite you at 3:00am, not about making you feel good,” he reminds developers.

• Remember that there are a million ways to get the wrong result, but only a few to get the right one. If you’re told to switch from the way that works for you to a way that works for someone else, just smile and do it. The most important thing is that it works.

• Pay attention to suggestions from those outside your area of expertise. They are there for a reason, and if they make you think about the larger world outside your current project, then you’ve just learned something that will prepare you to sit in their chair someday.

• If the DBA wants to talk about the data set size and the company standards coordinator wants to talk about the reasons you’re coding in C++ instead of Python and the processor freak wants to be able to export the code to run on an Abacus, have answers ready that cover the penalties and benefits of addressing his or her concerns. Anything that doesn’t kill your career will make you a better employee.

How do you know when your team’s attitude about code reviews is right? When the developer whose code was under scrutiny comes out of the meeting feeling better, not worse, with new knowledge and direction, says Madison Decker, senior technical lead at InfoCision Management.

But we’ve gotten this far, and we have barely gotten the developers into the conference room. What should they be doing once they’re in there? We cover that in the next section:

• 5 Reasons for Software Developers to Do Code Reviews (Even If You Think They’re a Waste of Time)

• What to Look for in a Code Review

• Making Code Review Software Tools Help, Not Hinder

• How to Lead a Code Review (let’s set up and lead the review)

• How Not to Run a Code Review

• Doing Spot-On Code Reviews with Remote Teams