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 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 anyway?"
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)
Reasons to Do Code Reviews
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 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 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, , performance, 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 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 , 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 , is for reviewers to challenge the design. "The design should have been ironed out in the 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.
For many teams, the code review cycle should start . 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 considers the code review to be particularly useful 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 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 guru who is now at working on the , 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 , such as block diagrams before code implementation and during implementation, according to developer Chuck Brooks.
Jason Cohen, founder of (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?"
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: and . 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.
To a great degree, this is about trust. How much can the developer trust others she works with to give her ? 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 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 . 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.
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 . 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.