How agile practices improve code review

Written on 9:27:00 PM by S. Potter

On a mailing list recently someone posed a question about how to incorporate code reviews into an "agile methodology". While I had questions about the way the question was posed it got me thinking about how agile practices make code review happen more often, more organically and more transparently. Having different members of the team review code at different times during the development process on agile projects is both explicit, but also implicit.

Values

Let us back up a little here. First off agile is at its core a set of four ideals. Prefer:

  • individuals and interactions over processes and tools
  • working software over complete documentation
  • customer involvement over contract negotiation
  • adapting to changing needs over completing a stale plan

Principles/Tenets

From these ideals there are about 10 or so principles or tenets that are most commonly discussed among agile practitioners from most of the main camps. This is where it starts getting controversial as some agile camps acknowledge some of these principles and others do not necessarily, so here are just a few that I want to talk about in relation to increasing code review on agile projects:

  • Collective team ownership of product: meaning everyone on the team owns the product and nobody on the team is exclusive to just one or a couple of components of the product.
  • Embracing change: instead of continuing down a set path determined long ago that no longer applies, the team needs to adapt to customer's changing requirements at set (and regular) intervals (these might be called iteration planning meetings/negotiations, for example). This has various implications throughout the iteration/sprint.
  • Verify expectations continuously: meaning whenever possible, without impeding development, verify that customer expectations will be satisfied and no new additions have negative impacts.
  • Customers set priorities: instead of technical staff or outsourcing companies specifying technical priorities, customers specify business/feature priorities.

Practices

This is the point where we start seeing the translation into agile practices, many of while you will start to recognize:

  • Pair programming which arose from both collective team ownership and embracing change tenets.
  • Test-first practice came about mostly to satisfy embracing change, but also verify expectations continuously
  • Assigning new stories independently of previous ones. This means that no developer or pair has a niche component or product area and is used to satisfy the first tenet listed above.
  • Continuous integration that helps satisfy the verify expectations continously principle.
  • No fear refactoring, which is only advisable if your test/spec suite has good enough coverage,. This means you will likely revise poorly constructed code on agile projects vs non-agile projects more often and have the chance to do it more often too.

Obviously with the pair programming practice that some agile methods promote, you have another set of eyes reviewing code as it is written. This catches errors closer to the time it was written, which is a big time (and money) saver.

Applying a test-first approach means that you start out defining custom expectations of the code first before writing the actual code. This is reviewing, perhaps not code in the traditional way, but the requirements, which in my view is even more productive usually.

When a project manager assigns new stories to a developer or a pair without confining him/her/them to specific areas of the codebase, this increasing the transparency of the codebase and exposes hidden dragons to be exposed before too long and hopefully before being launched into production (but you never know). Again this increasing the number of eye balls on parts of the codebase.

With continuous integration you can incorporate metric tools to capture some metrics of the code so that parts of the code that are obviously in need to technical refactoring get highlighted earlier rather than later. Increasing transparency of the codebase, which is always a goal of code reviews.

Hopefully you can see where I am going with this and not think about code reviews in the traditional top-down way where every month or quarter a piece of code is reviewed from each developer in a formal way.

There are a number of different agile (and common) software practices that could be used, I can think of 4 more off the top of my head that improves codebase visibility across more of the team.

If you enjoyed this post Subscribe to our feed

4 Comments

  1. gsporar |

    Nice post - I have some additional thoughts about code review aligning with agile at: http://blog.smartbear.com/the_smartbear_blog/2010/01/is-pair-programming-like-junior-high-sex.html

     
  2. Jesse Gibbs - Atlassian |

    Good post!

    A couple of comments/additional thoughts:

    Pair programming unquestionably helps devs produce better code. That said, almost every developer I've spoken to finds it impossible or impractical to pair program 100% of the time for a number of reasons:

    * Pairing can be 'intense'. Many folks I've spoken to find that working with another person is best done for shorter periods of time, say 1-2 hours. Interestingly enough, this was found to be the optimal amount of time for doing code review too (in the Smart Bear book. Good read!), so perhaps it has to do with how long a typical person is able to intensely focus on a given task.

    * With increased team distribution, sometimes members of the team are not co-located, which makes pairing impossible.

    Developers at Atlassian practice both pair programming, and what I like to refer to as 'continuous code review'. Just as continuous integration involves running fast builds for every commit, continuous code review involves frequently reviewing important changesets.

    Atlassian's Crucible code review tool and other products like Code Collaborator and ReviewBoard are good for supporting a continuous code review practice. These tools have the advantage of integrating with your SCM system such that it's easy to perform reviews based on changesets, either before or after they are committed to the repository.

     
  3. S. Potter |

    @Jesse, thanks for the Atlassian ad - I have always wanted one on my blog!

    Seriously though, I think advertising a tool that promotes an antiquated way of reviewing code in a non-agile way by saying pair programming isn't practical 100% of the time (to an agile crowd) to be a little silly.

    The blog post talks not just about pair programming. You should really be targeting non-agile teams with your product as it is tool focused, which as I mentioned in the blog post is anti-agile. So your advertisement is also badly placed.

    I do not mind you blatantly advertising your product, but when you make nonsensical and incomplete pitches this way, then you should expect this kind of response.

    If my post ONLY mentioned pair programming your advertisement pitch wouldn't be quite so annoying. However, you ignored 90% of my post and pretended you had actually read and considered the whole content in your response.

    Please do not post your advertisements to my blog again unless you are really responding to my blog post and can truly demonstrate relevance to the full content of my post.

     
  4. S. Potter |

    @gsporar sorry I meant to respond to you and say that I really like the Junior High School sex analogy!:)

     

Post a Comment