How to Address Your Coworker’s Bad Code

You’re sitting at your desk, trying to “track” down a bug that’s been reported, when it happens. The hunt takes you into some method that inspires you to do a double take. It’s about 1,200 lines long, it has switch statements nested three deep, and you think (but you aren’t sure) that it does the same thing two or three times in a row for no particular reason. You look at the source control history and see that this is another “Bob special.” After seeing this, you start thinking about finally having a long overdue talk with Bob so that you don’t have to keep cleaning up these messes. That sure won’t be a fun talk. So how do you approach it?

Philosophically Speaking

Let’s be clear about something up front. Getting really good at telling teammates that their code is littered with problems is like getting really good at breaking into your car after locking yourself out of it: it’s tactically useful in the moment but indicative that you need a better overall strategy. Your goal shouldn’t be to master gently telling coworkers about their bad code but rather to make the mastery unnecessary. And I say that not as some kind of meta cop-out, but rather to put your strategy into context as an attempt to start or further a relationship.

“Getting really good at telling teammates that their code is littered with problems is like getting really good at breaking into your car after locking yourself out of it.”
Tweet This Quote

When you’re part of a team, someone on your team who is committing bad code is a failure of everyone on the team—yourself included. So as you prepare for the intervention you’re planning with the person in question, keep in mind that you aren’t some kind of neutral crime scene investigator, sizing things up antiseptically. You’re part of the problem, and you share in the responsibility. Your team. Your code. Your problem.

The good news is that if you approach this conversation constructively, you’re taking the first step toward fixing the problem, the code, and thus the team. So the key is making it constructive.

5 Ways to Not Make Code Criticism Constructive

Before I go into detail about how to approach this conversation, I’ll give you a quick rundown of 5 things not to do.

• Don’t have the conversation when you’re frustrated or angry. Instead, wait until you’re calm and rational.
• Don’t get into this unless there’s a demonstrable problem. If you and he just have different casing preferences or something, the tension you create is probably going to nullify the benefits of standardization. Cosmetic coding standards and other relatively minor concerns can and should be addressed with automated static analysis.
• Don’t rely on seniority or status in any way. There’s no faster way to breed resentment than forcing people to do things they don’t agree with “because you say so.”
• Don’t expect to revolutionize someone’s entire approach in a single sitting and make the conversation a marathon affair. You want to have a clear and relatively concise message so that you get your point across without exhausting the other person. Improvement will happen over the long haul.
• Finally, don’t say that the code is “” That’s a useless, subjective way to categorize. Everything in software is about trade offs, so what you want to do is show Bob that he’s paying for quick and dirty coding with maintenance headaches for the rest of the team.

Build A Constructive Code Strategy and Environment

You’ve already prepared a bit by reading what not to do, so now it’s time to complement that with what to do. There needs to be three main components to this preparation:

(1) the gaps you want to address
(2) the support for your argument
(3) the outcome for which you’re hoping.

These three things are going to frame the discussion you intend to have.

The gaps are actual, specific problems with Bob’s code.

You don’t want to stroll over to Bob’s desk, pull up a chair, sit on it backwards and say, “So, Bob, you’re pretty bad at this programming thing…great talk!” You need to decide what tangible items you want to address during this discussion. What’s the most egregious source of problems? Is it the gigantic methods? The nested switch statements? The duplicate code? Pick one or maybe two of these things to cover. Just as you don’t want to be critical and vague, you also don’t want to be critical and devastatingly specific, reading off 95 of Bob’s greatest coding flaws like some kind of departmental Martin Luther. There may be 95 things wrong with Bob’s code. But if you want to fix all of them, it’s important to lay the groundwork for a mentoring relationship because you’re definitely not going to fix all of them in one day.

Building Support: Do Your Research.

Let’s say that you’ve decided to focus on method length as the topic to address. The next thing to do is build support for your argument. It’s a lot more credible to cite some supporting studies or widely respected industry figures on the matter than to march over to Bob and declare that his methods are too long. Build a case with evidence for the principle that you want to cover, and then also find specific, problematic instances in the code base to discuss. The last thing you want is to be hand-wavy about the problem—you want to be able to point at it and say, “for instance, this right here is a really big method.”

The Outcome Should Be Actionable

Having picked your issue and built a case, the last thing to do is choose an outcome toward which to steer the conversation. So you’ve shown Bob a giant method that he wrote and convinced him of the evils of giant methods. “Uh, okay,” he’ll say. “So what now?” Decide ahead of time that you want to work together to break the method into X number of smaller methods or that you want to leave the code in a state where no refactored method is longer than Y lines. Whatever it is, pick something actionable so that you and Bob can cap the conversation off with a joint win.

Be courteous

At this point, you’re ready to have the hard conversation.  If you do it right, it won’t be nearly as hard as you might think, and it will serve as a productive starting point for a series of subsequent conversations that will be easier and perhaps even pleasant.

This article originally appeared at SmartBear's Blog, written by Erik Dietrich.