4 ms·

You could simply go ahead and give the scathing code review, if you wanted. I used to work with a few senior engineers like that who were extremely sharp and,

by kirse 11y ago

You could simply go ahead and give the scathing code review, if you wanted. I used to work with a few senior engineers like that who were extremely sharp and, shall we say, point-blank with their feedback. But their feedback was always about the product, not myself, and they often provided guidance outside of reviews. After some initial butthurt, it teaches the young engineer to learn to separate their identity from the product they've produced. Or you could couch your feedback in mamby-pamby happy-land that said programmer still deserves a trophy for trying. I personally am fine with someone saying "this block of code sucks" if there's an unarguable set of reasons/metrics attached to that statement. Usually better if the reasons come before the conclusion. Avoid style preferences or bringing general identity accusations like "all your code sucks" into the review. Keep the focus entirely on the product not the producer and that's fair game. Maybe tell the newly hired coworker a story about your code once used to suck, and how you overcame that.

tomjen3 11y ago
Or you could avoid the fallacy of the excluded middle: list in a clear and objective way exactly what is wrong and (if applicable) points to fix it. "Sucks" is a subjective comment and not very useful feedback. I know my code sucks but not why it sucks - that is partly why it is being code reviewed.
- rancur 11y ago
  > fallacy of the excluded middle wordy synonym for false dichotomy?
  - jacquesm 11y ago
    No. X --- excluded middle --- Y Versus excluded left bit --- X --- excluded middle --- Y --- excluded right bit
HeyLaughingBoy 11y ago
This sounds like a list of excuses to be lazy and rude. It's quite possible to be extremely critical while remaining respectful. Of course, you avoid criticizing the programmer: that goes without saying. But saying "it sucks" changes the tone of the discussion to a confrontation and that is rarely the best way to handle a corrective situation. Point out the errors, introduce the dev to the quality standard expected by the team, offer help if needed. But treat people as peers, not as cretins that your manager forces you to work with.
fndrplayer13 11y ago
+1 Fantastic. Its hard to receive these reviews as a young engineer, and as a slightly more experienced engineer, it is also hard to give them. But its an important part of the job. Its fine, in my opinion, to give the hard review, as long as you walk over to your coworkers desk, and helpfully guide them through this as a learning experience. Having this stuff written down is also helpful for them to reason. That being said, I dont think I'd ever say, "This code sucks." I might say something like "I think there is significant room for improvement here if you were to <xyz>"
- rancur 11y ago
  room for improvement is still pointing out the failure to hit the mark. Much better to stay focused on positives in code, like you focus on the positives in a relationship. 'I wish you would...' is so much different from 'stop doing XYZ, it annoys me' I have only barely begun to understand let alone implement the relationship side, so I'm sorry I don't have more examples of how to handle this in code review.
rancur 11y ago
> trophy for trying trying should always be rewarded. It's the only way you get better. Raising children, chapter 7 [nods]. You're confusing 'trophy for trying' with 'trophy for half-assed effort'. Like with any child, you need to know the person well enough to know how hard they're trying. If you go by the value of the work they produce, you're a cold heartless father.

by kirse 11y ago

tomjen3 11y ago

Or you could avoid the fallacy of the excluded middle: list in a clear and objective way exactly what is wrong and (if applicable) points to fix it. "Sucks" is a subjective comment and not very useful feedback. I know my code sucks but not why it sucks - that is partly why it is being code reviewed.

rancur 11y ago
> fallacy of the excluded middle wordy synonym for false dichotomy?
- jacquesm 11y ago
  No. X --- excluded middle --- Y Versus excluded left bit --- X --- excluded middle --- Y --- excluded right bit

HeyLaughingBoy 11y ago

This sounds like a list of excuses to be lazy and rude. It's quite possible to be extremely critical while remaining respectful. Of course, you avoid criticizing the programmer: that goes without saying. But saying "it sucks" changes the tone of the discussion to a confrontation and that is rarely the best way to handle a corrective situation. Point out the errors, introduce the dev to the quality standard expected by the team, offer help if needed. But treat people as peers, not as cretins that your manager forces you to work with.

fndrplayer13 11y ago

+1 Fantastic. Its hard to receive these reviews as a young engineer, and as a slightly more experienced engineer, it is also hard to give them. But its an important part of the job. Its fine, in my opinion, to give the hard review, as long as you walk over to your coworkers desk, and helpfully guide them through this as a learning experience. Having this stuff written down is also helpful for them to reason. That being said, I dont think I'd ever say, "This code sucks." I might say something like "I think there is significant room for improvement here if you were to <xyz>"

rancur 11y ago
room for improvement is still pointing out the failure to hit the mark. Much better to stay focused on positives in code, like you focus on the positives in a relationship. 'I wish you would...' is so much different from 'stop doing XYZ, it annoys me' I have only barely begun to understand let alone implement the relationship side, so I'm sorry I don't have more examples of how to handle this in code review.

rancur 11y ago

> trophy for trying trying should always be rewarded. It's the only way you get better. Raising children, chapter 7 [nods]. You're confusing 'trophy for trying' with 'trophy for half-assed effort'. Like with any child, you need to know the person well enough to know how hard they're trying. If you go by the value of the work they produce, you're a cold heartless father.