In an ideal world, software developers look forward to peer code review. ?The practice helps them catch potentially embarrassing mistakes before they leave the dev environment. ?But beyond that, it also gives them an opportunity to showcase their work and to learn tips and tricks from peers. ?Again, in the ideal world, code review offers a uniquely efficient opportunity for personal growth.
Lest I seem too cynical, I’d like to point out that this also sometimes proves true in the real world. ?In healthy, high-functioning groups, team members enjoy and appreciate code reviews. ?They learn from one another and consistently treat one another like human beings.
But let’s be honest. ?For every healthy, high-functioning group out there, you’ll find another that epitomizes dysfunction.
Code Review Horror Stories: You’re Not Alone
Today, I’d like to offer both members and survivors of groups like that a bit of catharsis and empathy. ?It may seem bleak, but rest assured that you are far from alone. ?Others have code review horror stories as well.
Let’s take a look at some of them. ?These are situations I’ve either directly observed or composites of situations I’ve observed. I give you…
Everyone reading has heard of the death march, no doubt. ?This involves developers throwing themselves into implementation for prolonged periods of time, trying to help some late project wheeze past the finish line. ?Long hours, little sleep, and promises of good things when it’s over all abound. ?You grow to harbor an odd kind of love-hate relationship with the codebase.
We associate this with implementation — actually?writing code. ?But I’ve seen it for?reviewing code as well.
Some outfits have strict regulations and policies about reviewing every line of code. ?And some of those same outfits take a waterfall approach, wedging the code review phase between implementation and testing. ?(Okay, who are we kidding — in parallel with testing.) ?The result? ?A multi-day or even multi-week code review marathon.
Sit through one of those, and you’ll never want to see anyone else’s code again, yours included.
Next is something that has a similar flavor to our last code review horror story. ?The deposition style review doesn’t necessarily have to be long. ?But it?does have to be mind-numbing.
If the group is reviewing your code, they’ll bring a checklist to bear on it. ?And that checklist will contain the most mundane, easily automated minutiae imaginable. ?They’ll go through every field declaration you’ve written and manually check that an underscore precedes it. ?They’ll examine every variable deference to see if you’ve checked for null. ?And they’ll even check for tabs or spaces.
Before this gets even halfway done, you’ll be tempted to perform a table flip. ?By the time you’re finished, you’ll be numb and you will no longer care about anything. ?And after a few of these reviews, you’ll think of software development as a long, dry, legal-style process of initialing things and answering repetitive questions.
Let’s switch gears now. ?So far, I’ve talked about awful code review approaches. ?But now let’s look at some of the awful people that can populate them and make you hate your life.
First up, the dictator. ?You can probably imagine how this goes. ?You have one guy in your software group that rules with an iron fist. ?Probably an architect or tech lead, he wants no line of code in the codebase that he doesn’t personally bless. ?And achieving that blessing with even the slightest deviation from his personal preference? ?Good luck.
Code reviews involving a dictator start to resemble the medieval court of a mad king. ?Supplicants come, make their case heard, and try not to anger the dictator. ?They rarely succeed.
The In-Law Dinner
From a realpolitik perspective, navigating a dictator is pretty straightforward, if soul-crushing. ?But you can find yourself a character in a more complex code review horror story at review time.
Have you ever gone to an extended family function where people regard one another with somewhat-masked disdain? ?You don’t understand all of the backstories and undercurrents, but you do catch the occasional passive-aggressive jab. ?And you have the distinct feeling that things could devolve in a hurry at any time.
Code reviews can have that feel as well. ?In offices with lots of politics and bad blood, you can find yourself caught in far deeper currents than you can navigate. ?Are Sue and Dave?really that concerned about one lousy variable name, or are they fighting some older, more bitter battle by proxy? ?With this particular horror story, you get the feeling that the actual code seldom matters at all.
The Fraternity Initiation
As far as code review horror stories go, this one kind of seems like the opposite of the in-law dinner. ?There, you have petty feuds beneath a veneer of civility. ?With the fraternity initiation, you have rancor heaped atop a process that probably means well.
If you bring code to be reviewed here, you’ll find people directing insults and criticisms at you like crazy. ?What kind of idiot would extract a method there? ?Were you drunk when you named that test class? ?What’s wrong with you?
But in spite of the apparently mean-spirited nature of the comments, participants will tell you that everyone has thick skin and that’s just how the team expresses itself. ?And for some people, this might not be a horror story at all — they’d call it fun. ?But I promise you that, for some participating, it is most definitely a horror story.
The Backseat Driver
In its way, this one might demoralize participants more than any of the rest. ?I call this one the backseat driver because the “review” takes on a much different dynamic. ?Instead of reviewing the code, the reviewers start telling the reviewee every last detail of how exactly to rewrite the code.
People don’t come away from this style of review with a list of suggestions. ?Instead, they leave it with a detailed list of exactly how to implement things, how to structure them, and how to format them. ?The reviewee becomes a human keyboard through which the participants type.
People exposed to this develop a habit known as learned helplessness. ?It causes them to develop a habit of inaction in all situations in response to random, painful stimulus. ?The thinking goes, “I’ll get negative feedback no matter what, so why do anything?”
Avoiding the Horror
So what do you do? ?How does one avoid these and other horror stories? ?That could make for a post in and of itself. ?But you can get a lot of mileage out of two relatively easy policies. ?First, automate checks for everything that you possibly can in order to inoculate against marathons and depositions. ?And secondly, stipulate that developers treat code review feedback as suggestions rather than orders. ?Treat everyone as a professional by default.
Code review is far too valuable to eliminate, so you need to learn to optimize it for effectiveness and sanity. ?But you need to understand that it’s a highly collaborative activity that inherently involves criticism and contradictory ideas. ?If you have underlying personnel problems in your group, code review will bring them out in spades.
Automate what you can, and keep the activity positive and ultimately voluntary. ?This will help your code reviews a lot, and it will separate them from the deeper issues.