How do you tell someone they're writing bad code? [closed]

I've been working with a small group of people on a coding project for fun. It's an organized and fairly cohesive group. The people I work with all have various skill sets related to programming, but some of them use older or outright wrong methods, such as excessive global variables, poor naming conventions, and other things. While things work, the implementation is poor. What's a good way to politely ask or introduce them to use better methodology, without it coming across as questioning (or insulting) their experience and/or education?

Solution 1:

Introduce questions to make them realise that what they are doing is wrong. For example, ask these sort of questions:

Why did you decide to make that a global variable?

Why did you give it that name?

That's interesting. I usually do mine this way because [Insert reason why you are better]

Does that way work? I usually [Insert how you would make them look silly]

I think the ideal way of going about this is subtly asking them why they code a certain way. You may find that they believe that there are benefits to other methods. Unless I knew the reason for their coding style was due to misinformation I would never judge my way as better without good reason. The best way to go about this is to just ask them why they chose that way; be sure to sound interested in their reasoning, because that is what you need to attack, not their ability.

A coding standard will definitely help, but if it were the answer to every software project then we'd all be sipping cocktails on our private islands in paradise. In reality, we're all prone to problems and software projects still have a low success rate. I think the problem would mostly stem from individual ability rather than a problem with convention, which is why I'd suggest working through the problems as a group when a problem rears its ugly head.

Most importantly, do NOT immediately assume that your way is better. In reality, it probably is, but we're dealing with another person's opinion and to them there is only one solution. Never say that your way is the better way of doing it unless you want them to see you as a smug loser.

Solution 2:

Start doing code reviews or pair programming.

If the team won't go for those, try weekly design reviews. Each week, meet for an hour and talk about a peice of code. If people seem defensive, pick old code that no one is emotionally attached to any more, at least at the beginning.

As @JesperE: said, focus on the code, not the coder.

When you see something you think should be different, but others don't see it the same way, then start by asking questions that lead to the deficiencies, instead of pointing them out. For example:

Globals: Do you think we'll ever want to have more than one of these? Do you think we will want to control access to this?

Mutable state: Do you think we'll want to manipulate this from another thread?

I also find it helpful to focus on my limitations, which can help people relax. For example:

long functions: My brain isn't big enough to hold all of this at once. How can we make smaller pieces that I can handle?

bad names: I get confused easily enough when reading clear code; when names are misleading, there's no hope for me.

Ultimately, the goal is not for you to teach your team how to code better. It's to establish a culture of learning in your team. Where each person looks to the others for help in becoming a better programmer.