Team members discussing different solutions.
Code review is an essential practice for collective development. It improves quality — fresh eyes on a problem can pick up mistakes and flawed assumptions that individual developers, no matter how diligently they check their own work, will miss. A diversity of viewpoints, with different experience, knowledge and philosophies makes it more likely that the eventual solution to a given problem will be closer to optimal. Code review also allows knowledge to be shared within the team (not to mention across teams in the organisation, as we’ve found with our new #code-reviews Slack channel).
This communicative function of code review is perhaps the most important. Not just to pick up on mistakes, but to explain to one’s fellow developers the thinking behind certain decisions and why a particular solution has been chosen — which in turn can open up debate and build consensus on the direction of development.
I have sometimes found that the traditional code review process doesn’t allow me to fully communicate the reasoning behind decisions. I can explain my reasons for favouring one solution over another, in the pull request (PR) description, but the rationale behind some decisions is not easy to put into words. Borrowing a truism from creative writing, showing is more effective than telling.
Show, don’t tell
One way to show your thinking is through the commit history of a pull request’s branch. Reviewers can then see, for instance, a solution being tried and then abandoned in favour of better one.
But ultimately, what reviewers are presented with in a pull request is a fait accompli: this is the solution I have decided is the best, and you can accept or reject it (or make suggestions for improvement). What about decisions for which there is no obvious ‘best’ solution, or where there are arguments on both sides that could benefit from team discussion?
Of course, one could always talk it through with team members during development. But again, that’s telling, not showing. Someone who is not in the midst of the problem may not easily see the nuances that make a particular decision difficult to decide upon.
Pair programming is a popular and effective way of dealing with this kind of situation. It’s not always feasible, though: other developers in the team may be pushed for time, and although there are online options out there for remote collaboration, distributed teams might not find it easy to pair up on a problem.
Two PRs are better than one
I recently chanced upon another approach, which I’m calling A/B coding. I had a brief discussion with a teammate about the problem I was tasked with solving (duplicated records in a HABTM association, as it happens, though the specifics are unimportant here). I inclined in one direction, my teammate in another.
I was pretty convinced I was right, but I found I wasn’t able to articulate why I thought my preferred solution was better. Arguing in the abstract is sometimes a bit artificial. I decided to show the difference in approach, and the implications of each solution.
So I first implemented the solution my teammate suggested. (Crucially, I did this in good faith, trying my best to make the solution elegant, even though it wasn’t the one I favoured.) Then, in a separate branch, I implemented my preferred solution.
I then raised two pull requests based on the two branches. I labelled them option A and option B (with links in the PR descriptions) and I was open about which one I preferred. The option we chose would be merged, the other closed.
Concrete > abstract
As it turned out, showing rather than telling was effective: when he reviewed the two PRs, my teammate was able to see the superiority of my preferred solution. Of course it could have turned out differently, and we could have still disagreed. But even in that case, having the two PRs would have given us a practical basis for discussion: not ideas in the abstract, but realised solutions. If, for instance, I had not properly understood his preferred solution and implemented it inadequately or differently than he imagined, it would be obvious by looking at the code.
I’m not suggesting that A/B coding is applicable to every situation. Every development task involves a number of decisions, from the trivial to the highly significant; it would be impractical and undesirable to proliferate multiple PRs for every decision. And it wouldn’t be practical for larger PRs — but then, our PRs are always small and atomic, right? ;)
But I do believe that this method, used judiciously, can be an effective way of communicating thinking about controversial decisions. It can also help developers practically think through solutions and their implications. In some cases, I’ve found myself ambivalent about which solution to a problem would be best: A/B coding can help show that ambivalence and allow others to give their insights.
Another application of this technique might be turning pair programming on its head: after a discussion between two teammates who have opposing viewpoints on how a problem should be solved, for both to take on the same task, and individually implement their preferred solution. It would be imperative in this case to compare apples with apples, clearly defining the scope of the task, of course. But again, it’s easier and more effective to discuss realised solutions than ideal solutions.
All A/B coding, regardless of whether it’s carried out by one developer or two, is optimising for quality, rather than development speed. But as we are all aware, getting it right first time can save a lot of time compared to unpicking a sub-optimal solution later on.