How to criticize code

Dear new developer,

Criticizing code (and software solutions in general) is an important skill. It helps transmit norms, increase team knowledge, and improve solutions. But it isn’t something that comes naturally; at least, I had to learn how to do it. Similar to learning how to edit other’s essays, you must learn how to critique team member’s programming.

First, understand the context. There are two types that matter. The first is the business context. What problem is this system trying to solve? What limits does the business have (time, money, labor, understanding)?

In a startup, some of the code being written will be quick and dirty, because the startup is trying to find something that people will pay money for. Rapid exploration is more valuable than perfect architecture and maintainability.

You also need to understand the technical context. What is the scale/timeframe/level of the team’s knowledge? All of these things should inform your critique. I expect different things from an adhoc script in a data pipeline than I do from an embedded agent shipped to customers with little chance of future updates.

If you are part of a team, you may know these contexts, but they shift over time. So it’s always worth double checking before you begin a review to make sure you aren’t holding outdated assumptions. This can be as simple as a quick conversation with the author or team lead. It’s also helpful to regularly have these so that the entire team can be in the same mental space in terms of the engineering trade-offs they make daily.

Next, determine “which hill you want to die on”; that is, what is important enough for you to comment on. Which aspects of the code are important to you, based on personal experience, team goals, or ethical considerations? If you don’t know what the team values, ask! And then write it down so that others can benefit.

What you want to do with a review or critique is walk a line. You want to question and provide actionable feedback so that you and the author can improve the code and understanding of the system. But if you provide too much, the author may be overwhelmed or, worse, brush off your comments. You can ask what kind of feedback the author prefers; some folks have thicker skin than others.

Don’t focus on the small stuff. It feels like an easy win, but it really isn’t helpful. If you must give such feedback, preface it with “nit”. In fact, if you can automate away questions like spaces vs tabs or where curly braces go by using a linter that runs before a PR can be opened, all the better. Intent and high level feedback is where most of the value of code critiques lives.

Finally, a note on tone. You can be harsh with feedback, but don’t mix up someone’s implementation with who they are. They likely do that enough themselves; I know I sometimes get wrapped up in my code and take it personally when it is found wanting. The tone I find works best is one of assuming positive intent, but asking tough questions along the way. Trust that your team member has done the best they could, and try to focus on improving it and educating the both of you.

The end goal is to bring the code up to the current standards, or slightly beyond, of the codebase and team. Attacking anyone personally, even if they made a really ignorant implementation decision, actively undermines that goal in the future. I find that harsh feedback delivered in a kind tone is more effective than the reverse.

Sincerely,

Dan