9

How do you politely tell a colleague that his/her code is shit?

Comments
  • 7
    Politely point out what they can improve. Start with major stuff, leave lesser details for later. Don't make it personal, focus on the code and avoid generalizing expressions.
  • 15
    "well that's some spaghetti right there!"
  • 11
    You hire two other people, have your colleague stand between them and then you give the two cards that say: "Congratulations! your code is not shit."
  • 1
    We can still be polite while being straight on it... I would prefer that. .Not much sugar chating required.

    At times a round table discussion on people bring things on table shamelessly also help...No one is good, no one is bad!
  • 6
    @netikras That or: "Dude, I had no idea you were Italian"
  • 5
    You introduce code reviews. Quickest and easiest way going to massively boost code quality from any commits, on all sides.
  • 1
    git revert <unwanted commit hash>
  • 2
    What @gronostaj said. Maybe focus on one thing at a time, and when they fix it, go to the next. Remember to praise them, otherwise some people get the impression that all you do is complain and point their mistakes, and then they get butthurt.
  • 2
    "Mr/Ms. X, I'm sorry but your code is full of shit, just like you."
  • 2
    No, srsly, why the puffy talk? Just point out what is the problem and get over it. If it's spaghetti - say so. If it's bad error handling - say so. If it's methods 20 stores high - SAY SO. And provide a detailed suggestion on how to fix it.

    I cannot stand the tip-toeing arround when there's smth so concrete that you want to say. I'm glad my mentor was going straight to the point every time. That saved lots of time and lots of effort I didn't have to spend on trying to decypher what the hell he meant with that puffy talk.

    If you _really_ need to be soft about it -- "does this work? Do all the tests pass? Good, it means you have a working implementation now. That's step 1. Now we/you need to clean your code up, bcz it's a pile of spaghetti. These parts should be grouped into a single service, this does not belong in a controller, and you should never ever use a repository in your controllers, so that method must descend down to service layer. Write abstractions for your components and maintain the SOLID compliance. Keep the tests coverage over 85%, preferrably ~95%. When you think you're done - gimme a shout, we'll see how close are you to the finish line"

    being straight does not mean being a jerk
  • 1
    Don't be afraid to teach someone something useful. the way I usually do this is saying something like "yes, this will work, however if X happens this will fail" if its a logical problem and then suggest how to do it better. If its a code style problem Id remind them that if I have trouble decyphering the next guy will too, and the same goes for them when they'll need to fix it months down the line. And again suggest what I think is the best practice and why I think its better. Hell if Im not sure about parts of my suggestions I'll even remind them that Im also interested in their opinions in case they have a better idea!

    Just treat it as a learning experience for both of you and you can't go wrong. Be a teacher and a student at the same time
Add Comment