Editorial Note: I originally wrote this post for the SmartBear blog. You can check out the original here, at their site. They have a number of authors over there that are writing good posts.
Asking someone to look over your work is a pretty common practice in any discipline. We’ve been conditioned with the idea that it’s always good to have a second set of eyes look at your work prior to officially marking it as done.
Writing and reviewing code is no exception. It’s pretty easy to forget to check an argument for null, so having someone review your work only makes sense. In fact, it makes so much sense that software departments tend to formalize and mandate the process, calling it “code review.” And the practice is quite effective.
But formalizing and mandating a process in a corporate setting can lead to a variety of unintended consequences.
Office Politics of Code Review
I often quip that if an endeavor has more than two people, then it also has politics. Furthermore, if you think that it doesn’t, you’re likely starting off at a disadvantage because you’re playing a game without realizing that you’re playing. In the office setting, where money, power, and influence are at stake, politics tend to be magnified.
If your department has code reviews, then your department has politics around them. The ideal state is that these politics are minimized to the point of being unnoticeable, but that is often not the case. In fact, they’re often so noticeable that they’re toxic. Is that the case with your department?
Let’s look at some types of personalities you may have to deal with in the office and ways you can avoid office politics anti-patterns around code review.
“The Big Man on Campus”
The “big man on campus” (BMOC) is probably the least intricate political situation imaginable, though it has the potential to be among the most counterproductive. In this situation, there’s a pushy personality that cows other group members into coding according to his will. Usually this personality tendency is reinforced by organizational seniority (and not necessarily competence), with the BMOC having a title like “tech lead,” “architect,” or, at least, “senior software engineer.” However, that need not be the case; the BMOC can simply be a particularly aggressive and domineering team member.
You can recognize this political situation and the crux of the problems that it causes by the words and attitudes of other team members. “Oh, Bob’s not going to like that. You’d better take it out.” The best case scenario is that the BMOC is actually quite sharp, technically. When that’s true, he’s just a bottleneck — a micromanager that inspires learned helplessness in the team. The end product will be good, but the effort won’t scale well. The worst case scenario is that the BMOC is technically incompetent and rules through pushiness alone. When that’s the case, you still have the learned helplessness and the scale problem, but the work product is also poor.
“The Cool Kids”
If the group with a big man on campus is a monopoly, the cool kids anti-pattern is an oligopoly. The software group is not ruled by a single dictator but by a cartel of established, senior people. The cool kids don’t necessarily agree on everything, but they agree on a lot of things, and they tend to keep their infrequent disagreements in-house. The reason disagreements are seldom and private is that retaining their status as members of the in-crowd is more important than being right and more important than the software. A cool kid might go along with something like global variables not because she thinks that they’re a good idea but because the other cool kids might gang up on her and excommunicate her if she goes against them on the point too often.
This anti-pattern is most recognizable by new developers always, somehow, being wrong about everything. A newly-hired developer is often a goldmine of fresh perspective, but the mine will remain untouched if the cool kids rule the roost. Admittance to the inner circle is hard-won and requires a lot of invested time and proved loyalty. In the meantime, good ideas and practices are discarded if they come from the wrong people. The danger with this anti-pattern is that it promotes coding monoculture and limits ideas to the most long-tenured and thus the most insulated people on the team.
If the cool kids split the group into the insiders and outsiders, the tournament ladder dispenses with the ability for people to rest on their laurels. Like its namesake sports structure, the person on top can be displaced at any time, and all competitors can move up or down. The most important thing here is being right about any detail, however obscure or trivial. You can move up the ladder by proving that your deserialization scheme shaves five microseconds off the existing one. You can move down the ladder by being wrong about some arcane piece of compiler trivia.
You’ll know you’ve wandered into a tournament when every code review seems to give rise to competitive sharpshooting of anyone’s claims and constant, obsessive research to prove one’s correctness. People will spend hours preparing their defense of their code ahead of review. At first blush, this seems like the sort of healthy competition that drives everyone toward improvement, but that turns out not to the case. In the first place, it makes everyone neurotic and is hard on morale. And, even more importantly, the focus isn’t on delivering value through software or even getting the software right because the software is really just the tournament venue. Rising on the ladder and defending against upstarts is all that matters.
Avoiding It All
These are far from the only political arrangements that might crop up in your group if you aren’t careful, but they’re the most common ones that I’ve seen in my travels. What you’ll notice is that, while the game changes in each scenario, the fact that the game trumps the well-being of the software is shared by all scenarios. The more politics there are around writing and reviewing the code, the less productive the writing and reviewing will tend to be. You need to minimize it. Here are some ways to do that.
- Ensure that reviews are two-way. Never have people who only review and people who only get reviewed.
- Always focus on the code and not the person who wrote the code.
- Make the reviews small, frequent, and informal. Marathon group sessions in rooms make people defensive.
- Frame things as questions and suggestions rather than orders and accusations. Ask that others do the same.
- Automate as many checks as possible so that reviews don’t focus on simple details.
But, in my experience, the single most important thing you can do to cut down on politics is to make taking the feedback as optional. You don’t “pass” or “fail” code reviews, and you don’t get your code “approved.” Instead, you ask feedback and take it when you find it valuable. That preservation of autonomy is absolutely essential to having a good, collaborative environment.
Removing the gatekeeper nature of a code review may seem risky, like leaving your house unlocked. And, I’ll concede that a stubborn developer leaving a defect in the code is a risk. But it’s nowhere near the level of risk that you run when your code reviews are more about politics than about improvement. That is a risk that you cannot afford.