5 ms·

> What's wrong with giving exactly that feedback? Giving feedback on a code review is really meant to improve small bits here and there. Incremental improvemen

by maratd 11y ago

> What's wrong with giving exactly that feedback? Giving feedback on a code review is really meant to improve small bits here and there. Incremental improvement. If someone doesn't know a new language construct, a better way of doing things, etc. If someone simply writes crap, telling them that they wrote crap isn't helpful ... because they're clearly incompetent and are only capable of writing crap. > If your managers pressure you or your coworkers to "go easy" on the quality (ie. "It works, let him check it in!") then again you have a big problem. This is really the issue. First, you'll piss off the incompetent coder. Second, you'll stall the project. Third, you'll piss off your boss, since he's probably the guy who hired the incompetent coder. All the while, you're "making waves". All based on what is essentially your opinion on what clean code should look like. Because it does run. I would just take it slow and incrementally target some of the worst offenses. You'll never make the guy into a competent developer, but you can at least ameliorate some of the worst offenses and improve the quality of the code. If you're actually working on the codebase as well, instead of just reviewing it ... I would talk to management. Explain the situation. They're either going to fire the new guy or you're going to need to find another job. One of the reasons I like freelancing is that I don't have to face these dilemmas. The next client is just around the corner.

rancur 11y ago
> you're making waves as long as nobody makes waves, we'll have a peaceful experience until the shitstorm of the reality of unmaintainable code hits. this is one reason why companies need to die, and not be bailed out
curiousDog 11y ago
> If someone simply writes crap, telling them that they wrote crap isn't helpful ... because they're clearly incompetent and are only capable of writing crap. I think you can't reach such a conclusion without knowing the circumstances. What if he's a fresh college grad or a junior developer? I think you're coming from a start-up environment where everyone around you is supposed to be an expert in their field. Most Big Cos hire fresh college grads and mentor them. So mistakes like this are common. Now considering he's a junior, Remus's advice is spot on. I helped a dev on our team in a similar way who went on to write some really complicated distributed systems code that you use today. Also Remus comes from MSFT where this advice would apply.
- maratd 11y ago
  > What if he's a fresh college grad or a junior developer? Even when I was in college, I didn't write crap. Obviously my code wasn't great, but it wasn't crap. And there is a lot of crap out there, I see it every day. Based on the OP's description, we're talking about someone who isn't capable of producing quality work. You can either write code or you can't and it is very obvious by seeing the code. If the person can write code, but needs a few pointers, that's what code reviews are for. If they can't write code, they need to move on to a different profession ... or move up to management.
  - DigitalJack 11y ago
    That's BS. Coding isn't some kind of magical incantation, in spite of the jokes in SICP. Anyone that can understand basic algebra can learn to code.
    - woah 11y ago
      At some point, there needs to be some sense of "if you're not going to do it right, don't do it at all". Anybody could give a haircut, but do you want a bad haircut from someone who does not want to improve?
  - MaulingMonkey 11y ago
    > Based on the OP's description, we're talking about someone who isn't capable of producing quality work. Ehh - I've seen coders who can produce quality work, but sometimes choose not to because of (perceived) time pressure or (perceived) lack of value. Their mental model for code generation is to generate crap, then identify the patterns, then refactor until it's not crap - where the last two steps are "optional". Time pressure: A new hire likely feels some pressure to prove they can be productive. Value: New grads may not be in the habit of maintaining code (having written lots of throwaway assignments), not so new programmers may be used to writing throwaway prototypes, or "code ownership" situations where readability by coworkers isn't valued. Heck, it's quite possible they realize their current code is crap, and may even plan to clean it up in a subsequent changelist. I do similar on solo projects & private branches for the sake of keeping 1 changelist ~ 1 change parity, as it makes it easier to figure out refactoring mistakes. That said, those changes should generally be either done before the code review request, or noted as intended followup - and some pushback is warranted.
koala_man 11y ago
My favorite and severely underrated code review trick is positive reinforcement: "Oh nice, a cleanly separated MakeSnafucated function! I'm working on document level snafucation, and just being able to call this will save me a lot of time! Thanks!" If the coder has any potential, not only will they write more modular code like that in the future, but they'll do so happily rather than complain to management about your so-called perfectionist nitpicking.
swsieber 11y ago
"Reprove with sharpness, then show afterwards an increase of love." Telling a coder he needs to break down a function and get rid of deeply nested function isn't calling it crap, it's giving him a way forward to improve his code. As a new hire, he deserves that feedback. If he follows it, compliment him and give positive feedback. Whether a person can become a good coder isn't determined by how he writes code, it's how he reacts to the feedback he gets. If he can take correction, he can become a good coder.
- sarciszewski 11y ago
  > Whether a person can become a good coder isn't determined by how he writes code, it's how he reacts to the feedback he gets. Quoted for emphasis. My first recursive postgresql function was crap. I recently learned that it became a DoS vector at 3 layers deep because I had horribly flawed logic. I spent an hour the other night refactoring it. The cruelest feedback comes not from mortals, but from the harsh production environments which do not tell you what is wrong, but merely go offline.

by maratd 11y ago

rancur 11y ago

> you're making waves as long as nobody makes waves, we'll have a peaceful experience until the shitstorm of the reality of unmaintainable code hits. this is one reason why companies need to die, and not be bailed out

curiousDog 11y ago

> If someone simply writes crap, telling them that they wrote crap isn't helpful ... because they're clearly incompetent and are only capable of writing crap. I think you can't reach such a conclusion without knowing the circumstances. What if he's a fresh college grad or a junior developer? I think you're coming from a start-up environment where everyone around you is supposed to be an expert in their field. Most Big Cos hire fresh college grads and mentor them. So mistakes like this are common. Now considering he's a junior, Remus's advice is spot on. I helped a dev on our team in a similar way who went on to write some really complicated distributed systems code that you use today. Also Remus comes from MSFT where this advice would apply.

maratd 11y ago
> What if he's a fresh college grad or a junior developer? Even when I was in college, I didn't write crap. Obviously my code wasn't great, but it wasn't crap. And there is a lot of crap out there, I see it every day. Based on the OP's description, we're talking about someone who isn't capable of producing quality work. You can either write code or you can't and it is very obvious by seeing the code. If the person can write code, but needs a few pointers, that's what code reviews are for. If they can't write code, they need to move on to a different profession ... or move up to management.
- DigitalJack 11y ago
  That's BS. Coding isn't some kind of magical incantation, in spite of the jokes in SICP. Anyone that can understand basic algebra can learn to code.
  - woah 11y ago
    At some point, there needs to be some sense of "if you're not going to do it right, don't do it at all". Anybody could give a haircut, but do you want a bad haircut from someone who does not want to improve?
- MaulingMonkey 11y ago
  > Based on the OP's description, we're talking about someone who isn't capable of producing quality work. Ehh - I've seen coders who can produce quality work, but sometimes choose not to because of (perceived) time pressure or (perceived) lack of value. Their mental model for code generation is to generate crap, then identify the patterns, then refactor until it's not crap - where the last two steps are "optional". Time pressure: A new hire likely feels some pressure to prove they can be productive. Value: New grads may not be in the habit of maintaining code (having written lots of throwaway assignments), not so new programmers may be used to writing throwaway prototypes, or "code ownership" situations where readability by coworkers isn't valued. Heck, it's quite possible they realize their current code is crap, and may even plan to clean it up in a subsequent changelist. I do similar on solo projects & private branches for the sake of keeping 1 changelist ~ 1 change parity, as it makes it easier to figure out refactoring mistakes. That said, those changes should generally be either done before the code review request, or noted as intended followup - and some pushback is warranted.

koala_man 11y ago

My favorite and severely underrated code review trick is positive reinforcement: "Oh nice, a cleanly separated MakeSnafucated function! I'm working on document level snafucation, and just being able to call this will save me a lot of time! Thanks!" If the coder has any potential, not only will they write more modular code like that in the future, but they'll do so happily rather than complain to management about your so-called perfectionist nitpicking.

swsieber 11y ago

"Reprove with sharpness, then show afterwards an increase of love." Telling a coder he needs to break down a function and get rid of deeply nested function isn't calling it crap, it's giving him a way forward to improve his code. As a new hire, he deserves that feedback. If he follows it, compliment him and give positive feedback. Whether a person can become a good coder isn't determined by how he writes code, it's how he reacts to the feedback he gets. If he can take correction, he can become a good coder.

sarciszewski 11y ago
> Whether a person can become a good coder isn't determined by how he writes code, it's how he reacts to the feedback he gets. Quoted for emphasis. My first recursive postgresql function was crap. I recently learned that it became a DoS vector at 3 layers deep because I had horribly flawed logic. I spent an hour the other night refactoring it. The cruelest feedback comes not from mortals, but from the harsh production environments which do not tell you what is wrong, but merely go offline.