Pair programming and code review, singly or in combination, are among the best investments any enterprise can make in strenghtening developers' skills, developer team cohesion, and the quality of the code. The two practices serve many of the same purposes. They are similar but distinct, so I am covering both in one article.
This article describes my approach to pair programming and code review: if you want a historical overview or more about the variety of possible approaches to this, there is a Wikipedia article on each topic.
In pair programming two developers sit together (or Zoom with one another, or whatever) and actively interact while writing code. Code review is a similar process for code that is already, at least in the coder's view, completed. Most often two or more developers go together through code that one of them has completed quite recently. Code review looks for comprehensiblity, coverage of edge cases, conformance to standards, and even outright bugs; participants usually also discuss how it might have been done differently, or how they'd approach a similar problem next time.
Pair programming necessarily involves exactly two people: any more than that and it's just a meeting, not a hands-on programming session. In its most orthodox form, only one of the two actually writes code, while the other is more of an interlucutor. Code review can involve more than two people — I've had my code reviewed in a conference room by four or five developers — and it can also be done by a single developer on another developer's code in the latter's absence, with comments and questions simply written into the code and passed back to the original developer. (I've even similarly reviewed code left behind by someone who had already left the company in question.) That last approach requires that the reviewer be a very good writer.
Code review can be "driven" by the coder or by an interlocutor; it depends who is more comfortable running what is essentially a meeting with a simple agenda: go through the code; understand it; one hopes, improve it. Usually, code review adds a lot of comments but code as such is not immediately changed, or only trivial code changes are made: if code changes will be needed, whichever participant is hands-on at the computer puts a specially marked comment in the code and someone — typically the developer who wrote the code — gets back to it later under less pressure, to actually fix it.
My rule of thumb is that about 10% of hands-on programming effort should be devoted to pair programming and/or code review. (The emphasis there is on "hands-on programming": the amount of code review time is independent of time spent writing specs, working out system architecture, testing, etc.) I have yet to see a project where that investment didn't at least roughly pay back within two release cycles.
Both pair programming and code review should generally be peer processes. The process should work equally well regardless of whether the reviewer or the person whose code is being reviewed is the more "senior"; similarly for pair programming. It is generally a bad idea for someone's manager to review their code, unless it is basically a lesson in how to review code. (Reviewing your manager's code, on the other hand, can be an interesting exercise.) The goal of these processes is to improve the code, not to judge people. If a team is tiny, it is usually a better idea to get review done by someone outside the team than to fall back to having it done by the manager (again, excepting training exercises).
Among the many benefits of pair programming and code review:
More maintainable: Unless a body of code is a throwaway one-time task, it will eventually need to be maintained (bug fixes, new features, changes to third-party APIs, etc.). It typically takes an enormous amount of experience and self-discipline for a developer to have any idea what is obscure to others in their code.
Convergence: I worked for several years in a group of developers on a big telephony and messaging system, with certainly over 30 person-years of developer time invested into that code base, plus commensurate QA work, hardware integrations (phone switches, voice cards, fault-tolerant RAID disk systems), etc. We did so much pair programming and code review that by the end you could almost never tell whose code was whose unless they put their name on it: about the only clue was how good the writing was in the comments. We ended up with common conventions for naming variables, identical rules for code layout, a strong consensus on what needed commenting, and a constantly strengthening system architecture because we got consensus as to what parts of the system were becoming top-heavy (too many new "features" on too little initial architecture) and needed to be revisited.
That proved to be a very good thing for our employer, because (back to "maintainable") over five years later in a series of mergers and spinouts, that code base constituted a major asset which got sold to two different firms on a mutually non-exclusive basis. One of those firms had pretty solid continuity of developers, but the other needed only a few person-months of consulting from members of the old team to get a new group of a dozen or so developers, testers, etc. completely up to speed.
Van Gogh could not have painted this.
O'Keeffe could not have painted this.
Strengths and weaknesses: Developing coding skills isn't like learning to beat a 1980s arcade video game, where everyone is more or less on the same path and it's just a question of how many tricks you've learned. It's more like becoming an artist: Van Gogh couldn't paint a Georgia O'Keeffe painting of a flower, and O'Keeffe couldn't paint a Van Gogh. One developer may be great at meticulous detail; another at devouring a new API; yet another at getting a good overview of the system, or seeing just where to abstract a parameterized function so you don't have to write almost the same code fourteen times. Someone may actually like maintainance programming and making incremental improvements; they might be the only one on the team who does. Someone else might be much better at keeping up with the "latest and greatest" extensions to a computer language or function library, another might read StackOverflow the way the average person binge-watches a mini-series; yet another might be great at coming up with names for variables.
And (back to "convergence") some of them will learn a lot from each other. It will surprise a non-developer to learn this, but in the absence of processes like pair programming and code review, few developers ever look at all closely at colleague's code unless and until they have to change it to fix bugs or add features. Developers talk to one another about database design, about neat new language features or an API they just found, about system architecture, etc., but in the absence of overt encouragement, they don't talk about, "Here's how I handle having multiple loop variables" or "Here's how I decide what information to put in an error message."
Learning the whole system: Pair programming and code review are both really great ways to expose different developers to parts of the system they might not otherwise ever look at. Under Agile approaches where there is relatively little by way of technical specifications, this can be particularly important. (Yes, you should retroactively write the equivalent of a technical specification, and I've done a bunch of those, but how many companies ever really allow time for developers to do this? Pair programming and code review may not solve that problem, but they mitigate it.)
Team-building: Properly approached, pair programming and code review can really help to build a jelled team. If there are tricky egos involved, definitely start with pair programming. It's a lot easier in human terms to get two developers to cooperate with one another than to have one later critique the other's supposedly finished code. If there are no tricky egos involved, code review is probably better "bang for the buck," because you never have one person sitting there while the other hems and haws for several minutes thinking about something.
Bug fixes: A lot of people are under the misimpression that code review is about finding bugs. Code review will probably incidentally find some bugs. It is pretty much the only way other than extensive white-box testing, which few companies do, to find a bug in code that implements a capability that has no features built upon it yet. Ditto for "time bombs" that work fine now but which make an assumption that will someday be false. However, the purpose of code review is much more to make sure that code is readable and conforms to standards (or close enough that no one is bothered) and to let developers see what each other are doing than to find bugs. (Want to find bugs? Much more important than code review is to put together a good test team; ideally, have at least one person on it capable of whole-systems thinking and another person with a hacker mentality who likes to work out a way to break your lovely new system. Also, put your Beta in front of prescisely those end users who will pound it mercilessly and expect it to work perfectly. They may not look like they're your friends, but they are.)
If your development team doesn't regularly review code and/or occasionally use pair programming, I can pretty much guarantee that you'll gain a lot from introducing these practices. They have enough in common that for the rest of this article, I will just focus on code review.
The key to making code review succeed is to have is a mandate from management: "Yes, it's OK with us that some time be allocated to this. Yes we understand that this is an investment in better code and a more mutually connected dev team." If management treats code review strictly as a cost (or, worse yet, as an indulgent waste) it probably won't happen, and certainly won't happen right.
It is probably best to start with relatively senior developers' code being reviewed by less senior developers. This won't necessarily have the most rapid initial payoff in terms of improving code (though you just may discover some surprising talents among your junior developers) but it is far easier socially than the other way around.
A few things to keep in mind when first doing code reviews:
>>>00002: Error not getting logged
>>>00003: Not the data we need in the log
>>>00007: Vestigial, kept for the moment as harmless, get back to this
>>>00012: Bad variable name(s): misleading, uninformative, obscene, or inappropriately multiplexed
>>>00021: Scalability issue
>>>00026: Outright bug
>>>00002 JM 2021-02-04: need to log error
>>>00007 JM 2021-02-04: Vestigial, keeping for now
>>>00003 JM 2021-02-04: if this error pops, we need to know what file(s) it affected: add that to the log. Probably also who (if anyone) is logged in
>>>00003 JM 2021-02-04: May want each caller to this function to pass in a unique ID so on error we can log that and have context. Or have this trigger a stack dump
>>>00003 JM 2021-02-04: Database error, so log the failed query (and maybe the immediately preceding query as well, because its return is how we built this query)
>>>00026 This function pretty much explodes on a negative value of 'count', so it needs to check its inputs better.
$_REQUEST. If the code in a particular file calls a function that accesses
$_REQUEST, then an implicit parameter, possibly quite an important one, may not be referenced overtly even once in the present file. Reviewing these, or generating them during review, can be as important as reviewing the code itself.
for-loop behaves like a normal
for-loop or that
total_cost = cost_per_hour * hoursmultiplies
hoursand stores the result in
total_cost. Presumably, code is going to be read only by someone who can read code.
My e-mail address is email@example.com. Normally, I check this at least every 48 hours, more often during the working week.