How do I approach a coworker about his or her code quality? [duplicate]

Question

I work on a new venture at a large enterprise software company (3000+ programmers). In my group, we have a bunch of projects and people usually work on several projects over the course of a year.

I just started work on a project that has been previously maintained by a buddy of mine (consultant who's been with us for 3+ years) to add some features. I got into the code, and the quality was really quite poor. Whether it was on the UI frontend or the services backend, the code simply wasn't indented, there were hundreds of lines commented out for no apparent reason, documentation was basically non-existent, coding standards weren't applied consistently (e.g. mixing camelCase and under_scored_variables), variable names were unintelligible, datatype choices were wrong, etc. etc.

I'm very much a non-confrontational person, so I don't want to attack my coworker, but I also don't just wanto to go to my boss and complain about his performance. What are the kinds of things I could say to politely mention that the code is poorly structured?

EDIT:

I want to clarify that while I understand there is an element of "Everyone else's code sucks" to all programmers, when I see something like this (names are chosen on purpose and some details left out/changed in this example):

public void doCalculate(Object argument) {
  if (argument instanceof String) {
    String argument2 = (String) argument;
    if (argument2 == "DataBase") {
      // do something
    } else {
      long argument3;
      try {
        argument3 = Long.parseLong(argument2);
      } catch (Exception e) {
        argument3 = -1;
      }
      // do something completely unrelated
    }
  }
}

I think it's objectively fair to say that this is not a good idea. Furthermore, I'm not dealing with a newbie here (I'm only coding 3 years now). He's got maybe 20 years of experience on me. The advice you guys have given so far is great; just wanna make sure that we're not talking about a "fine line" here.

Answer 1

Ask him to explain his code to you

Tell him you've never seen X programmed that way before, and ask him why he codes it that way. Show him the way you code it, and tell why you do it that way (best practices, better performance, less chance of errors, easier for other programmers to read/maintain, etc).

Be sure to prepare all your arguments in advance, and focus on why your method is best instead of why his method is worst. Afterwards, see if he still supports his method over yours.

If he is open to improvement, he will likely change his way of coding. If he still prefers to use his style of coding over yours, you are not likely to change his opinion.

---

This is the exact same answer I gave for the question How to tell your boss that his programming style is really bad?. I originally voted to close this question as a duplicate, however enough people thought it was different enough to warrent re-opening it. My answer is still the same though, regardless of if you're talking to a boss or a co-worker.

Answer 2

What if instead of approaching him personally, you approach the team and in a completely neutral way propose on "team" coming up with a coding standard/guidelines as a way for different team members to work more efficiently with each other's code.

Once that is in place, I think the code you have concerns with will bubble up to the top all on its own.

Peer code reviews also help in this area. Coding standards don't have much benefit if no one ever looks at the code. But peer code reviews have many other benefits, primary one knowledge transfer, so you should push to introduce them in either case.

I'm making an assumption that even if you have 3000+ engineers, you guys have MUCH smaller sub teams that work together as a unit

Answer 3

Your co-worker is probably not the root cause of the problem in your company. The reason that poor quality code gets into production is the lack of automatically measurable coding standards in your company. In this case, sunlight is the best medicine.

You should talk to your technical lead about implementing automated source code quality metrics in your group. The system should run at least weekly, e-mail everyone on the team a file-by-file list of coding standard violations, and send an executive summary to your boss. The summary should contain the number of violations per project.

In my experience, anything that gets measured and reported to a boss improves over time.

Answer 4

...the code simply wasn't indented, there were hundreds of lines commented out for no apparent reason, documentation was basically non-existent, coding standards weren't applied consistently (e.g. mixing camelCase and under_scored_variables), variable names were unintelligible, datatype choices were wrong...

As far as I can tell, above matches #12, #9, #5, #2 and probably #7 of Top 12 things likely to be overheard if you had a Klingon Programmer (archive) [original link]:

12. Specifications are for the weak and timid!
11. This machine is a piece of GAGH! I need dual Pentium processors if I am to do battle with this code!
10. You cannot really appreciate Dilbert unless you've read it in the original Klingon.
9. Indentation?! - I will show you how to indent when I indent your skull!
8. What is this talk of 'release'? Klingons do not make software 'releases.' Our software 'escapes,' leaving a bloody trail of designers and quality assurance people in its wake.
7. Klingon function calls do not have 'parameters' - they have 'arguments' - and they ALWAYS WIN THEM.
6. Debugging? Klingons do not debug. Our software does not coddle the weak.
5. I have challenged the entire quality assurance team to a Bat-Leth contest. They will not concern us again.
4. A TRUE Klingon Warrior does not comment his code!
3. By filing this PTR you have challenged the honor of my family. Prepare to die!
2. You question the worthiness of my code? I should kill you where you stand!
1. Our users will know fear and cower before our software! Ship it! Ship it and let them flee like the dogs they are!

It is quite likely that the only way to avoid confrontation is a warp drive into another space quadrant.

Answer 5

First, everybody thinks other people's code sucks.

Second, maybe he inherited the code from someone/someplace else OR this was a prototype never intended to be the code used for the actual project but of course that's what ended up being used.

Third, maybe his code really does suck but is it your job to badmouth previous developers? Unless you have built up a reputation that commands respect around the company I suspect if you do whine about the other person's code it is you that will end up looking bad and not your friend. The time for giving people grief about their code is when code reviews are held. At least then it is part of your job responsibilities.

I recommend that if you don't like his code then fix it to your liking. Then the next developer will come along and complain about your code and probably change it back to be more like what your friend did.

Answer 6

Get some background about what was going on within the company and the team when this project was written. Ask this dev for some feedback on how he thought the project went. Has his style changed? If given the opportunity, what would he do differently?

He may agree with your standards, but just didn't have the luxury of applying them during the creation of this app or just didn't know about them.

Either your group has no standards or code review policy or a serious problem with enforcement. You may be holding an opinion of one.

Answer 7

I'll admit it, I am sometimes that guy. When I am, I will have my reasons, usually time constraint, death-march projects, instructions such as "now, not perfect" etc. Very rarely, it's because of a bad day. I am happy to get asked about any issues, it gives me a chance to become a better developer. Provided a) You are reasonable about what will be changed (ain't that badly broke, agree to don't fix), and b) your not a nut job dictator who believes your way is the only way.

To approach me about it, do not ask "Why". Why is a fighting word, a challenge, and is ultimately destructive. If you find yourself using Why too much, change it to I. "Why did you call this x" becomes, "I would have used index, it's more descriptive than x."

The best way is to state what you would have done, or would have preferred. Explain that what you are seeing does not met certain standards, and that you would like to see it improved. My guess is that 99% of the time, the response will be "so would I, but........"

Answer 8

It is unlikely that the author of the code will come back to fix his code for you out of pure good will, or that your employer will pay him to do that. So, either way, you are scr*wed.

If you believe that the mess the code is in will have a severe detrimental effect on your performance while working on it, be sure to save a backup of the original code somewhere so that you can prove that it was a mess when it was delivered to you. So, if your employer confronts you later about your poor performance, you can cover your ass.

Still, it may look to your employer like an excuse, so try to avoid even that happening if you can.

Answer 9

Think about what good mentioning him that he has a bad coding style would do you or him?

Perhaps it's better to try to improve your company's QA procedures.

Answer 10

You need to find out two things: What is your goal? How would you like to be approached yourself?

If your colleague, with 20+ years of programming behind him, is really, really open-minded, you can just go talk to him and point out specific examples, tell him why they looked wrong to you and discuss alternatives. Because he's really, really open-minded and keen to learn, he'll listen, a discussion will ensue, and he'll understand your points and you'll both live happily ever after.

In my experience, though, the colleague is most likely programming like that because he just doesn't care about style. It could be because he's really in the wrong job, or it could be because, in 20+ years of programming, he never found a reason to worry about it - maybe his code just works and he moves on.

If the latter is the case, you're just going to waste your and his time, and you may make a dent in your relationship if it turns into an argument. So what is your goal here?

Do you want him to clean up his code so that you can work on nicer code? Do you want him to learn and improve? Do you want to feel good about being a better coder? Do you want your boss to know that you're a better coder than him?

The first one is not going to happen and the third is already established, although you probably want him to recognize it as well - good friend, father figure?

If you really want him to learn and improve, then figure out how you would like to be approached in the reverse situation, and do the same. And figure out what would annoy you, and make sure you don't do that.

The forth one is easy: either you want a better salary, and then you just demand it during the next feedback meeting (it doesn't really matter how good you are in this industry) or you want more responsibility and then you just keep doing a good job until you get it.

Answer 11

Suggest you boss that you have been seeing some crazy code and ask him to put all possible staff on some programming courses. I think he should be aware that his apps are being filled with crappy code.