We recently had the chance to catch up with Gregg Sporar, Senior Product Manager at Smart Bear Software, about peer code review as an Agile process.
DZone: Can you give us a quick intro to yourself and your background?
Gregg: My background is in software development, which is what I've always wanted to do. I've worked on a variety of applications, ranging from control software for a burglar alarm to 3D graphical user interfaces, and I've always had an interest in the processes involved in creating software. For the last several years I've been focused on software development tools, first at Sun Microsystems where I worked on NetBeans, and now at Smart Bear Software where I am the Senior Product Manager for our code review tool, Code Collaborator.
DZone: What makes peer code review an Agile process?
Gregg: If you look at the benefits of peer code review, they align really well with the Agile Manifesto and the principles on which that manifesto is based.
The main benefit that aligns with Agile thinking is that code review helps teams get to working software sooner. Many studies have shown that peer code review is an effective way to find defects. It is an important part of building quality software, along with things like unit testing, static analysis, and continuous integration.
It is interesting how much time we spend during an iteration discussing the requirements with our stakeholder(s) and the emerging design and architecture with other software developers, but when it comes time to actually write the code, the tendency is for each developer to work in isolation. The interactions during discussions of requirements, architecture, and design uncover flaws. The same principle applies to the writing of the code: if team members collaborate during code creation, they will write better code.
A secondary benefit that aligns with Agile is that code review helps enable sustainable development, which can be pretty scary when you think about it. Your team members have different areas of expertise. How much are they sharing that knowledge and bringing each other up to speed?
This concept is usually expressed as a "bus number." For each part of your source base, how many members of the team would have to step off a curb and get hit by a bus before a part of the code is no longer maintainable? That's the "bus number" for that part of the code and if it is less than 2, you've got a problem.
By encouraging the team to read and discuss the source, code review helps maintain collective code ownership, increasing the bus number for the reviewed code. That way, if a team member is on vacation, or leaves the team, progress can continue at the same pace and the sustainable development goal is met.
Check out Gregg's video on Peer Code Review as an Agile Process from Agile Austin.
DZone: If there is resistance to introducing code review on an Agile team, how can that be overcome?
Gregg: The number one piece of advice I give to teams is to start slowly. Most teams fail if they take the approach of "starting tomorrow, we will code review every line of the source." It's better to instead work code review into your team's process a little bit at a time, typically by just reviewing a subset of the source.
Perhaps just review changes to the stable branch (depending on your branching strategy). Or have the team put together a list of the "10 scariest files to change" and just review changes to those files. Another option is to start out by just reviewing the unit test code - over time you can work up to reviewing the actual implementation.
The goal is to get some "quick wins" that show the benefits of code review without being too disruptive. As the team gets used to the rhythm of putting code review into their work flow, you can increase the amount of code being reviewed. We have a white paper that covers this topic: 11 Best Practices of Peer Code Review.
The other key to code review success is to make sure everyone understands that the goal is to review the code not the coder. This really means two things.
For developers it means using the correct tone when providing review comments; no flaming! Comments should invite discussion. For example, instead of commenting with "you should use this other API, not that one," ask a question: "What was the reason you chose to use that particular API?"
For managers it means not using code reviews punitively. If you tell a developer, "I see from the code review statistics that you have introduced more bugs than your co-workers" then the process will fail. Code review metrics are not useful for evaluating software developers. More on this topic available here.
DZone: What are some best practices for implementing Agile peer code review?
Gregg: First of all, find the right technique (or combination of techniques) for your team. Four basic approaches work well in Agile environments:
- Over-the-shoulder. When you need a review, find someone, go over the code together, and get his or her feedback.
- Email. Send out the files to the reviewers and then carry on the discussion in email messages.
- Pair Programming. Write your code in pairs. This provides a "continuous code review," since there are two additional eyes watching all the time.
- Tool-Assisted. Automate the manual bits of gathering files, tracking feedback, etc. with peer code review tools.
Once you have a technique, you need to make sure you get the best return on the time that developers spend doing code review. The founder of Smart Bear, Jason Cohen, did a study at Cisco Systems that came up with some great guidelines. You can find further details in a free book you can get from our web site, but some of the highlights are:
- Limit the amount of time – you should spend no more than 60 to 90 minutes at a time doing code review.
- Go slowly – typically 300 to 500 lines of code per hour is the maximum rate for an effective review.
- Limit the amount of code – as a product of the time and rate recommendations, the total amount of code for a review should be no more than 200 to 400 lines.
Beyond the results of that study, though, there are additional guidelines. A key one is that if you decide to use review checklists, keep them short. Remove obvious items from the checklist like "verify the comments match the code." Focus instead on things that are easy to overlook, such as verifying that all error conditions are handled correctly.
A final consideration is whether to do code review before or after the developer commits the changes to version control. An obvious advantage of pre-commit review is that you know that all files in the version control repository have been committed. An advantage to post-commit reviews is that the developer can easily move on to other tasks while waiting for feedback.
As with so much of software development process, you have to make some trade-offs. But if you invest in finding the right technique for your team, peer code review pays big benefits.