Practice3.Pair Programming and Code Reviews

Practice 3. Pair Programming and Code Reviews

An important practice to prevent defects is to ensure that someone else looks at new code before it is checked in. This is true whether it's a bug fix or a part of a feature and regardless of the amount of code changed, even if it's only a single line! The main reasons to have someone else look at new code are to ensure that:

• More than one person understands the code. XP rightly calls this "collective code ownership." If more than one person understands the code, it means that there are many people who understand the overall behavior of the system. This is very important to defect prevention, because any added knowledge in the team of overall system behavior is a good thing and will help everyone on the team think ahead and be able to more accurately predict where future problems might occur or where new changes conflict with or duplicate already existing code.

• Shared code understanding also means that it is easier for team members to pass their knowledge on to someone else, such as new team members. It also prevents people being "locked" into one section of the code or being a single point of failure because they are the only ones who understand it. This is a bad thing; not only for the individual's career but also for the company (what happens when the key person get sick?).

• Ruthless testing is employed. Part of the job of the second person is to think about the tests and ensure that the tests cover all the important cases. In many ways, the reviewer can look at the "fitness" of the solution just by knowing what the user problem is and examining the tests to ensure they meet the desired criteria.

• The new code is not an ugly workaround and does not duplicate any already existing code.

• The new code does not contain any obvious logic errors.

There are two primary ways to ensure someone else looks at code changes: pair programming and code reviews. Unfortunately, most software texts emphasize code reviews, but the optimal approach should actually concentrate on pair programming, with code reviews as an adjunct or outlet.

The primary advantage of a code review is that it gets a number of people together to discuss some code. Learning happens and good ideas are exchanged. However, code reviews are actually very poor at finding logic errors unless an inordinate amount of time is spent in the actual reviews, and this extra time is in my opinion hard to justify unless you are writing software where there is the potential of a catastrophic effect if the software fails (such as a person being killed). The problem with finding logic errors is that the best time to find them is while the code is being written; when presented with a mass of code as in a code review the logic and order behind the creation of the code is no longer evident. Hence, pair programming.

Pair programming is simple. Two programmers work at the same computer on the same problem at the same time with one monitor, keyboard, and computer. Pair programming is much more effective than code reviews at finding logic errors and preventing defects because both programmers will bring their shared and differing perspectives to the problem. In fact, all the benefits of having someone else look at code before it is checked in are maximized with pair programming. Additionally, pair programming is an excellent means to introduce new team members to the software by pairing them with a more experienced team member. There is no better way to learn a software system than to help solve real problems in it.

The only problem with pair programming is that it is a very intense experience that can be stressful and possibly even demotivating for one or both people in the pair. It isn't for everyone and it shouldn't, in my opinion, be viewed as something that is done exclusively all day, every day, by every member of a software team.

The benefits of pair programming are clear. But what isn't talked about, except by those who use pair programming as an excuse to not use XP or agile development, are some of the drawbacks of pair programming. For the "observer" in the pair, it can be annoying watching someone else type and make the same typing mistakes over (and over) again, type too slowly, jump around too much, and in short not have "control." For the person typing, it can be frustrating to have someone interrupt your train of thought or make suggestions that are on a wildly different line of thinking. More serious drawbacks of pair programming show up when there is a personality conflict in the team. Although these situations are relatively rare, they cannot be ignored because they can be very disruptive to the team.

I think the optimal approach to ensuring that someone else looks at every software change is to use a mix of pair programming and lightweight code reviews. Pair programming should be used as much as possible, but with an available outlet to work independently with joint code reviews when required. The key thing is to keep the code reviews lightweight using some of the following suggestions:

• It is critical to follow the "continuous integration" practice outlined in Chapter 4 on Working Software. Changes should be small and easily described. Don't wait days or weeks or months before having code reviewed, because that will require a great deal of effort to review and minimize the benefits of the review.

• Given small changes for review, exchange them frequently in e-mail or have someone drop by for a quick walkthrough.

• Avoid needless documentation and "ceremony" for the review. Keep it simple enough to ensure that the benefits are gained.

• Buy a projector for your team and hold group review sessions with critical code and its associated tests. Try to keep these fun and focused on benefits such as learning, not on trying to find "mistakes" and "errors." Rely on the tests to find the major errors, so your team can derive the most benefit.