Candor and fairness
In the interview I recently published on this site, production designer John Lavin talked about separating yourself as a person from things that you work on:
I used to teach drawing, and just the exercise of taking somebody who is not really used to that and saying “Here’s the drawing you made, go hang it on the wall and we’ll discuss it”, putting things up for criticism is something that I got pretty used to. And even though I’m sensitive to the criticism as a person, I’ve got a lot of practice with the idea that this painting, or movie, or thing that I work on isn’t me. It’s a thing I made, but it’s not me. It’s separating my identity from my product.
In Creativity, Inc. Ed Catmull writes this in the chapter “Honesty and Candor”:
[…] the only way to get a grip on the facts, issues, and nuances we need to solve problems and collaborate effectively is by communicating fully and openly, by not withholding or misleading. There is no doubt that our decision-making is better is we are able to draw on the collective knowledge and unvarnished opinions of the group.
And in the last post I talked about how important it is to focus on criticizing the code and not the person behind it:
Stay focused on criticizing code. If you feel strongly about something, be direct and concise. But never ever ever get personal. See the arguments for what they are and for what they should be. You are not your code. A critique of your code is not a critique of you as a person. Sometimes it’s hard to tell the difference especially if you’ve been working on something for a long time.
You can call it honesty. You can call it candor. You can call it sincerity. You can call it frankness. Some of these words come with heavy baggage attached to them, and that baggage can vary based on the cultural and societal aspects that you have absorbed in your life.
Clarity is the most important part I strive to achieve in the code I write. Clarity also plays a critical part in the code review phase, no matter which side of the process you fall on. In fact, let me strike that. In the code review phase it should not matter who wrote the code and who is reviewing it. You should work together as a team to crystallize the best solution possible to implement a specific feature or to fix a specific bug.
Communication within a team of people is already complicated. We all bring our own baggage, our own upbringing and our own cultural environment to it. Communication is by definition a two-way channel. Removing the complexity from this channel is the responsibility of both sides of the channel. Bringing clarity and candor to this channel is the responsibility of both sides of the channel.
As the person being requested to do code review, you should free yourself from the notion of how “senior” or “junior” the code author is. Bad code is bad code. Complex code is complex code. Criticize the code, fairly and constructively. The down side of not being as “senior” on the team is that you might not be aware of all the legacy aspects and decisions that were made when you were not on the team yet. It’s the responsibility of the code author to point you to the relevant decisions when you ask about them. The up side of not being as “senior” on the team is that you bring a fresh look. A fresh set of ideas. A fresh pair of eyes that should look at the code critically and see where, when and how it can be improved instead of being left to slowly stagnate.
As the person writing code and submitting it to review, you should detach yourself as a person from the product you are making. Free yourself from being defined by the code you write. Do not judge review comments based on how “senior” or “junior” that reviewer is. Free yourself from looking at those comments as something that originated from a person and treat them as a thing of its own. Some things that might be very clear to you would be too dense and opaque for a person who is not familiar with that part of the code base. Does it need more documentation? Does it need splitting up the logic into smaller, more manageable pieces? Or perhaps you unwittingly have fallen prey to the complexity beast?
The up side of being a “senior” on the team is that you are more aware of all the legacy aspects and decisions that were made since you’ve joined the team. It’s your responsibility as the code author to point the reviewers to the relevant decisions when you are asked about them. The down side of being a “senior” on the team is that it’s so easy to fall into the trap of been-there-done-that. It’s when the code base is slowly crumbling around you under its own weight, but you don’t even realize that it’s happening. That’s when you need somebody that brings a fresh look. A fresh set of ideas. A fresh pair of eyes that would look at the code critically and see where, when and how it can be improved instead of being left to slowly stagnate.
And if all the members on the team are candid and fair in code reviews, you might all just have a chance of enjoying working on that code base. At least most of the time.