Pair Programming Code Reviews
Pair programming can be effective, but can a pair be trusted to review their own code or should it be done by an independent third party?
Our company, Appresso Corporation, has been using pair programming for over a decade in building products such as DataSpider Servista, enterprise middleware which can enable IT teams to integrate a broad range of data sources without the need for coding.
In this article, I’ll share what we’ve learned so far. We produce several enterprise software products, so what we’ve experienced and learned may not be directly applicable to your situation, but hopefully it will stimulate some ideas.
First, for those unfamiliar with the approach, let me throw in a definition.
What is Pair Programming?
Pair programming is an agile software development technique. The most common scenario is one in which two programmers sit and work together at one computer. However, some do it remotely. We did ours in-person, so that’s the scenario I’ll be describing.
There are two roles:
- Driver: This is the one doing the actual hands-on writing of code.
- Navigator: Sometimes also referred to as the “Observer,” reviews each line of code as it is typed in.
The idea is that, while reviewing, the navigator should be steering things in the right direction, coming up with ideas for improvements, and anticipating future problems. This frees the driver to focus on the tactical aspects of completing the task at hand. In this way, the navigator serves as both a safety net and guide. It’s common for the two programmers to switch roles frequently.
Before we get into the issue of code reviews, which is the main focus of this article, let me first share what we’ve discovered from our experiences with pair programming.
What We Discovered
It’s Tiring
In pair programming, the Driver is continuously coding while being watched by the Navigator. When I served as the Driver, I didn’t even have time to glance at my phone notifications. Even during the compiling time, when I would usually be able to take a break if working solo, I had to use the time to explain details and discuss the next steps with my Navigator.
We usually work on a mutual task with our partner for about an hour before switching to the next item. Sometimes, we’ll take a break during the hour, but usually we carry on working, and take a ten-minute break before starting the next task.
It’s intense. We give it everything we have and at the end of each hour, are feeling spent. So, it’s important to take a break to recharge before tackling the next task.
Obviously, pair programming only works if the Navigator is clear on exactly what the Driver is doing so that they can advise on the next steps to be taken. So, the Driver must constantly be checking-in with the Navigator.
It Encourages Focus
When programing alone, it’s harder to know if you are really writing optimal code, specifications, and making the right design decisions. You’re fully responsible for your own trajectory and it’s not always easy to know if you’re on the right track.
During pair programming, as the Driver, knowing that I am being watched, I feel compelled to write the best code I am capable of. I’m much more focused because I don’t want to embarrass myself by writing sub-par code.
It’s a Lot of Fun
More than anything, pair programming is fun! As programmers, we want to develop useful and high-quality software. If the Driver gets off-track or slips into doing sub-par work, the Navigator is there to correct and guide them. Knocking out tasks one-by-one, knowing you’re doing your best work, and moving in the right direction is a greatly satisfying experience.
Leaning in to Pair Programming
When a large team works with the Scrum framework, pair programming tends to occur naturally. This is because, even during a one week sprint, increasing the work in process is not effective.
For example, if six programmers each take on a different task at the same time, they will end up following different trajectories. The easier tasks will be completed quickly, while the more complex ones will require more time.
If a developer takes a day off, nothing happens. To prevent these problems, at least some members of the team will begin pair programing naturally at times.
Increased Productivity
According to the research presented in Laurie Williams and Robert Kessler’s book, Pair Programming Illuminated, the development work time increases by about 15%.
In other words, if something takes twenty-hours to do by yourself:
- Pair programming will take 20 * 1.15 = 23 hours.
- This works out at 11.5 hours per person.
- The time for actual work will also be 11.5 hours.
We haven’t been carefully tracking our working time per task, but my gut tells me that this is about right. When pair programming, things progress faster than when working alone. While total hours spent increases by 15%, the development period is shortened.
Improved Quality
Pair programming is a terrific way to enhance information sharing and improve code quality. While time spent increases around 15%, a skilled Navigator helps by finding and pointing out new things as the pair progresses and each day brings new discoveries. The bandwidth of knowledge sharing is increased and the final product is superior to what a solo coder would produce.
While I’m confident that I do a decent job when working alone, sustaining a high quality level is harder, and having to come back and fix issues later is a pain. In contrast, with pair programming, it’s easier to maintain code quality, and we can continue pressing forward with confidence.
It’s Not the Only Way
I don’t want to over-hype pair programming. While it does have its merits, don’t expect it to be a magical cure-all — especially in emergency situations.
While we’ve used pair programming for around ten years and usually find it to be a productive way of working, it’s far from the only approach we use. In fact, if it was the only way we worked, it would probably introduce various problems.
Pair programming is not always convenient. Sometimes it just doesn’t fit the situation. Furthermore, not all developers enjoy it, and perhaps some don’t have the right personality fit for it. Pair programming should be just be thought of as one of many approaches that a development team can choose.
Ok, now that I’ve set the stage, let’s dive into the main topic of this article.
Pair Programming and Code Reviews
Reviewing on GitHub Enterprise
Before, when we used Apache Subversion (SVN), we’d have the committer (reviewee) sit together with the reviewer and walk them through the code. Once the review was over and any issues were resolved, we’d commit the code.
These days, we use GitHub Enterprise and make a pull request to initiate a review. Pull requests let you tell others about changes you’ve pushed to a GitHub repository. Once a pull request is opened, you can discuss and review the potential changes with collaborators and add follow-up commits before the changes are merged into the repository. We have much respect for SVN, but have found GitHub Enterprise suits our current working style better.
We found that pull requests had the following advantages for us:
- Can be reviewed asynchronously.
- The context of the review remains as a comment.
On the other hand, we discovered the following issues:
- It tends to be unclear when the code will be reviewed or merged.
- Larger pull requests are tough for the reviewer if they must look at the code from scratch and try to figure it all out on their own.
We dealt with these issues by attaching a document explaining the code design, having the reviewer speak with the coder, and sometimes having the coder and reviewer sit together to go through everything.
Our team originally began pair programming along with implementing the Scrum framework and found that it makes reviews easier. We haven’t properly monitored our development activity, but I’d say that these days, around 80 to 90 percent of the code we write is done with pair programming. However, since we started doing it in an ad-hoc manner, we continued working on Pull Requests in the traditional way.
Choosing the Reviewer
Obviously, the code reviews should be undertaken by someone other than the one who wrote the code. However, once we got into pair programming properly, the question arose, “Would it be more efficient to have the pair review their own code?” And, “If we do that, can we really consider it a proper review?”
We came up with a compromise: For each story (pull request), we’d have two reviewers, one of which could be the Navigator from the submitting pair, but not the Driver.
In short, we found it to be very efficient. Since we wrote the code as a pair, the Navigator who participates in the review knows it well, and so things proceed smoothly. Contrast this with reviewing unknown code alone, which is stressful and time consuming because you must figure out the code by yourself.
However, this begs the questions:
- Does code written in pairs even need to be reviewed? And,
- Is there any point in having a pair review a pull request?
While we can consider pair programing a continuous review, however, based on our experience, a separate review via pull request is necessary.
Of course, the Navigator would ideally direct the Driver carefully and follow the cycle of “red, green, and refactoring.” However, this is not always the case. Both the Navigator and Driver are only human. Either or both may make errors that are missed. For this reason, we make sure to review code written in pairs so that we can catch these missed errors.
The focus and courage which are encouraged by pair programming can sometimes discourage comprehensiveness or sensitivity.
Rotating Pairs for Review
We found that rotating pairs for reviewing code made our team stronger. Over time, each team member was exposed to every part of the overall project and saw the methodologies being used. We were not only able to take a fresh view of each part, but also spread know-how throughout the team.
This means that usually, we have anywhere from two to five team members’ user accounts involved with each pull request.
An example of a pull request with relatively more members.
As a rule, we change pairs one person at a time (one person leaves and the other remains). However, we still find that knowledge gained from our work is fragmented.
For example, if while a Driver was working, they decided to come back to a task later, they might forget once they change pairs and get focused on a new set of tasks. This can result in items being missed, or in duplication of code. This kind of problem can be prevented by reviewing code before merging since the duplication is usually easy to spot.
It’s tempting to think that since code was produced by a par that it is fine, however, if may result in merging duplicated parts. It’s always better to take the time to properly review the code to avoid missing mistakes or duplicates. A review is also a good opportunity to know how effective the pair programming has been and whether they have been covering all angles or not.
Even with pair programming, having a code review of a pull request provides a good opportunity to get a wholistic look at the work being done. Thus, from experience, even when most of the code is written by a pair, we have found that code review is useful.
Pairs Reviewing their Own Code
I question whether doing a pull request and review within a pair is sufficient since they were both involved in the work and are unable to see it with a fresh pair of eyes.
When you write code in pairs, even if you were the Navigator and not the Driver, you’re prone to be attached to the work. It’s hard for programmers to throw away the code they’ve written and this is also true for pairs. You’re always going to be conscious that “this is code I wrote.”
However, when someone else reviews, they are seeing “code that someone else wrote”, and can take a more critical view. They can try running the code, and carefully go through each line, checking to ensure that standards are being followed. In this way, tens of comments can be made.
While it’s hard to quantify, I think that when a pair reviews their own code, they miss out on having this level of objectivity. For this reason, we believe that it’s always best to have someone who had no participation in the coding process do the review.
How Extreme Programming Affects Code Reviews
We practice extreme programming (XP), a type of agile software development methodology intended to improve quality and responsiveness to changing customer requirements. It advocates frequent releases in short development cycles to improve productivity and introduce checkpoints at which new customer requirements can be adopted. One elements of this is pair programming and doing extensive code reviews.
By rotating programming pairs and reviewers, we are exposing everyone to the overall code base. They start to regard all the code as their own. Mob programming is when the whole team works on the same thing, at the same time, in the same space, and at the same computer — we’re somewhat doing this, but with time lags.
The good side to this is what we have many brains involved in the process of writing code combining their intelligence. However, we should think about having people who are not directly involved with the code. If we really do have a “co-ownership = one brain” situation, then even if we have up to five people reviewing a pull request, the review is no longer objective.
This makes me wonder if it is better to increase the number of eyes for reviews to make our collective brain smarter. If we do have a collective mind, then there will be friction between those who are directly involved and those who aren’t.
Perhaps a bit of friction is a good thing. If the friction can be classified as a coding issue, it probably will be more efficient to solve it in pair work rather than in asynchronous code reviews. On the other hand, if the friction results in finding bugs due to different viewpoints, then it is worthwhile having an “outsider” involved in the review process.
Summary: Be Mindful of the Effectiveness of Any Approach
One thing we have learned over the years of trying different approaches is that just because code was written via pair programming does not give an accurate indication of whether a review is warranted. Similarly, due to rotating teams and everyone developing a feeling of complete code co-ownership doesn’t necessarily mean that the necessary friction would be absent if we reviewed our own code.
There are many factors that can act as variables to affect the result including the skills and personalities of the members involved, and the characteristics of the products they were working on. There are a broad range of factors that can change the state of a team at any given time.
To properly answer the question of, “What type of code review is most effective?” you would need to monitor these myriad variables and analyze them to find trends. This would certainly be an interesting exercise that would result in many useful insights.
However, without such analysis, it is difficult to give a simple and definitive answer to the question, “Is it necessary to review even if pair programming?”
From all our team’s experience with the Scrum framework we have not been able to come up with a clear conclusion as to whether reviews are necessary when doing pair programming. The best answer we can give is that it depends on the circumstances.
Currently, while our team takes a case-by-case approach, we tend to have pairs do their own reviews among themselves. However, this may change in the future.
Therefore, I would like to conclude this article by saying that it is important to be aware of the effects of any approach you take to development, including pair programming. Collect as much data as possible and monitor your activities. But, at the same time, be sure to listen to your intuition.
About the Author
Kotaro Noguchi works in the team that builds DataSpider Servista, enterprise middleware which empowers IT teams to integrate a broad range of data sources without the need for coding.
