Agile Zone is brought to you in partnership with:

Lyndsey has posted 139 posts at DZone. View Full User Profile

What Makes Peer Code Review an Agile Process?

04.08.2010
| 18759 views |
  • submit to reddit

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:

  1. Over-the-shoulder. When you need a review, find someone, go over the code together, and get his or her feedback.
  2. Email. Send out the files to the reviewers and then carry on the discussion in email messages.
  3. Pair Programming. Write your code in pairs. This provides a "continuous code review," since there are two additional eyes watching all the time.
  4. 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:

  1. Limit the amount of time – you should spend no more than 60 to 90 minutes at a time doing code review.
  2. Go slowly – typically 300  to 500 lines of code per hour is the maximum rate for an effective review.
  3. 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.

AttachmentSize
gs350.png17.2 KB
Published at DZone with permission of its author, Lyndsey Clevesy.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Bino B. Manjasseril replied on Thu, 2010/04/08 - 11:32am

What do you think of doing the code review as a team? For example, all the developers gather in a room and the person who developed the code projects it on the screen and everyone get a chance to review the code and offer comments right there.

Gregg Sporar replied on Thu, 2010/04/08 - 1:09pm

@Bino: Well... it's okay for some teams, but it's not my favorite way to spend time.  In any case, there are some keys to making that approach work.  First, everyone at the meeting needs to arrive well prepared.  That means: read the code ahead of time and come with your comments and questions - watching people read code is not a good use of time.  Another key is to focus on finding problems and *not* on discussing solutions.  If you dive into the discussion of the solution, you're getting off the track of code review.

Carol Mcdonald replied on Fri, 2010/04/09 - 10:33am

Thanks Greg !

Jonathan Crossland replied on Mon, 2010/04/12 - 7:22am

This is a good example of riding on the back of agile methods. Code reviewing is a helpful activity, but it is not agile. Also code reviewing is a context driven activity, you cannot just have code reviewing as part of every day activity.

Code reviewing that occurs infrequently with certain 'under achieving members' can be perfectly acceptable and welcomed, but code reviewing that gets discussed with a team becomes unhelpful and adds weight and cost if :

- the team is not suffering from really bad code anyway.
- the team members are all capable and the standards are good to great
- the reviewing/reviewer is not subjective
- the 'improvement' is negligible


Most of the time, a developer or two who are 'under achieving' can be motivated to improve by a large range of other methods.
I think code reviewing is more useful with inexperienced teams, but 'design workshops', 'best practice workshops' and other positive enforcement techniques are much more beneficial than scrutinizing individual mistakes or subjective improvements.

Gregg Sporar replied on Mon, 2010/04/12 - 1:38pm

 

@Jonathan

 One of the strengths (and weaknesses) of "agile," is that everyone gets to have their own definition.  I was pointing out that peer code review aligns well with the manifesto and some of its underlying principles.  To me, that makes it agile.  

Your statement about "you cannot just have code reviewing as part of every day activity" might be correct for your team, but not all teams.  We have it as part of our every day work flow on our team.  And we're not the only ones - I spend almost half my time talking to software developers and more and more they mention the importance of code review in their standard practices.

Limiting code review to just "under achieving" team members has some drawbacks:

1. You don't catch errors made by the "best" team members.  Yes, even the super stars sometimes make mistakes.

2.  You don't share knowledge about the code amongst the other team members.

3.  You don't get a chance to learn good ideas from the code that was implemented by the better members of the team.  

In other words, code review is not just about finding problems in the implementation.  It's also about sharing ideas and praising the exemplary work of others.

 

Jonathan Crossland replied on Thu, 2010/04/15 - 9:27am in response to: Gregg Sporar

@Gregg I argue that the benefits decrease with "experienced" team members, if you hold it often (especially daily), not that its not beneficial at all. The benefit must be measured by the time/cost of the time to actually do the review and how much people are getting out of it. I don't think it's fair to say that code reviewing only adds benefit, it can easily become a negative or higher cost than benefit.

Gregg Sporar replied on Thu, 2010/04/15 - 10:48am

 

@Jonathan - Agreed, done incorrectly, the cost of peer code review can exceed the benefit. But I don't agree that the benefit decreases based on the experience level of the developers. I have not seen any evidence of that in our customer base, nor have I seen any studies that have produced that conclusion. More thoughts available on avoiding "incorrect" peer code review practices in our free book: http://codereviewbook.com/

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.