Avoid your initial reaction

Whether you are correct or not, your initial react has a fair chance of being wrong, overly emotional, or to be based on insufficient data. Even if you make a great and perfectly accurate snap decision, the speed of the decision will be taken as evidence that it is a bad decision.

Even if you are correct, you still need to spend some time thinking of valid reasons someone would write bad code, simply to be in a good headspace to correct a team member. Remember that if you had all the information to determine instantly whether a piece of code is bad in a given context, it’s because you wrote it.

Version control history is your friend. Make sure that the person you are blaming is the one that actually wrote it, rather than just the last to touch it.

Determine why the code was written that way

Most bad code isn’t written that way just because the developer is lazy, uninformed, or actively malicious. Something else is usually in the mix. You should leave your job if you start actively suspecting that this statement isn’t true.

Changing requirements and overly tight deadlines create a huge amount of bad code. Under pressure, many developers will write bad code that “works”, with the intent of fixing it later. But later never comes. Sometimes bad code is also written to blend in with other bad code in the system. Good developers are often forced to follow bad standards. The people in charge are not always the people who should be.

It’s also entirely reasonable for developers to have outdated knowledge. Things change quickly and dark matter developers (aka, people with social lives) aren’t living and breathing code 24/7. They miss things. The code might also not be bad – it could just be done in a way that you aren’t used to and may be that way for a good reason.

Have a better solution in mind

All the criticism in the world is useless when you can’t articulate a better solution. Find one or shut up until you can. This exercise also has another benefit. It forces you to give real (objective) reasons for why a piece of code isn’t optimal, instead of subjective ones. This has the effect of making discussions with reasonable people more reasonable and discussions with less reasonable people more obvious.

This is also a good time for you to critically examine your reasoning in regards to the “better” solution. If your solution is only “better” because it addresses potential issues that will never actually arise, it’s not better, it’s just “gold-plated”.

This is especially helpful when either you or the other developer are new to the team, as previous teams may have had a different set of constraints, which changes the meaning of “good enough”. This forces you to be able to articulate your reasoning and assumptions, so you can find out which are wrong.

Determine whether your better solution was actually workable at the time the code was written

With your “better” solution in hand, now you need to know whether that better solution was reasonable when the code was written. There may have been other constraints in place when the bad code was written (or potentially just stuff that the developer didn’t know), and it’s important to make sure you know what those were.

It’s a lot easier to come to somebody with suggestions for improvement when you can acknowledge that they didn’t have a lot of choice when they wrote the earlier code. A bit of empathy helps here. Further, if there is a repeated pattern of bad situations leading to bad code, being aware of it in general can help you avoid writing bad code yourself. It’s also worth bringing up to management if you can get away with it.

Figure out how you could refactor the code

If you still like your “better” solution, you need to figure out how to get there from here. Unless the code you are looking at is very small, this is something that will have to be done in stages. Figure out several steps to get the code into a better state, with the code being in a working, stable state at the end of each step (so you can be interrupted and return to it).

Most of the reasons that code is bad have to do with how that code interacts with other code in the system, either due to calling that other code or being called by it. As a result, you need to carefully consider how far out into the rest of the system your changes will spiral and make plans for limiting scope.

If you don’t have tests around the existing code, now is a good time to add tests for the existing version of the code. This lets you take smaller, more controlled steps when working on a fix with another person, especially if they don’t like the entire set of your suggestions for a fix.

Now approach the person at the right time in a non-threatening manner

Don’t tell them there is a problem in their code. Instead, ask if you can run something by them, or if they can help you understand something. It’s more important at this phase to keep from making them defensive than it is to be right.

Don’t do this when they have a looming deadline, personal/work drama going on, right before they go on / return from vacation, etc.. Don’t make this harder than it has to be by being oblivious to the political landscape.

It’s also generally best (unless there have been issues with this person in the past) to have this conversation in a one-on-one environment. A team environment is going to make people defensive at best and isn’t useful for this sort of thing. It’s also better to do this verbally, face to face, instead of over a chat client or as a comment in a pull request.

Remember to be respectful. After all, we all write the occasional bit of garbage code and when you talk to someone about theirs, you are training them in how to treat you when they find your mistakes.

Mind your wording

Avoid strong, subjective value judgements of someone else’s code. Calling it “terrible”, “garbage”, or “spaghetti code” may FEEL accurate (and we might agree with you), but it isn’t a useful way to actually get it fixed. Unless you are in a management role, your goal is to solve problems, not to browbeat other people into not causing problems.

Generally speaking, if you word your suggestions for improvements in such a way as to describe them as opportunities instead of improvements, it’s easier to get the other person on your side.

Also avoid accusatory wording. Asking what the reasoning was for a structure, rather than saying “why”. “Why” is more likely to provoke a defensive answer than something less subjective.

Work towards your answer without giving your answer

While earlier we suggested that you figure out exactly how you would do it, in general it’s not the best idea to simply show up with the fix. “I reject your code and replace it with my own” comes across as extremely cocky. And it’s a lot riskier if you are wrong.

Instead, ask questions to lead them through the thought process you had when you were looking at their code earlier. Not only is this likely to still lead them towards the same general answer, but it also makes it more likely that they will consider things in the same way in the future.

This also forces you to check your assumptions at each stage. An incorrect assumption early on can lead to a wildly different answer. If people initially get the impression that you are just throwing out poorly thought out solutions to get attention, it makes it harder to get your point across. This approach also keeps you from running roughshod over your more timid teammates, which helps avoid resentment.

Ask good questions

Be specific. Don’t ask “is this code good” – instead ask “is this code good for (x) situation”. Start with enough information to get an answer. It’s very irritating when the only answer is “it depends”. You want those variables out of the mix so that you can get a real answer.

Prefer questions with straightforward answers rather than essay questions. Answering questions is probably not high on someone’s list of priorities, so make it efficient for THEM if you want an answer. Make it obvious that you’ve put the work in to be able to ask the question. Remember that the person answering is also having to make assumptions about your knowledge level – you’ll get more useful answers if their assumptions are closer to correct.

What to do once you agree.

Just because you have a better way of doing something doesn’t mean you need to fix it right now. If your team is under deadline pressure, or has a lot of people working near the problem code, it’s usually better to leave it alone until that isn’t the case.

You also may have to agree to a partial fix to get rid of the worst aspects of the problem, with a better fix later. Don’t let a desire for perfect solutions stop good solutions from happening.

At some point, you’ll have to let management and product owners know about the problem so that you can make sure that they don’t find out the hard way. Depending on your organization, this might need to happen AFTER the fix occurs (especially if they are feature-driven and hostile to code cleanup efforts).