23

Intro: I’ve been writing software for over 15 years and currently lead a team of ~10 developers with whom I conduct code reviews daily. My comments are generally direct, honest, and straight-forward.

However, today's review had one comment in the mail that made few people pretty nervous; it goes like this:

If improving the code quality score means we have to introduce garbage like below, then we’re in serious trouble.

I was immediately pulled into a private chat session, where I was schooled on conduct and urged not to use unprofessional language, or else they would reply on top of my comment with a warning condemning unprofessional behavior.

What would you make of this; why do you think my language was unprofessional?

And yes, code was horrible, you can take my word for it.

Update 1: Full code review was detailed and constructive, with suggestions on how to improve it and how to avoid such mistakes from Code Quality tools.

Update 2 (after a long time): I keep seeing the same problem every once in while, but let me be very clear on something: I should have mentioned before that "garbage" word I used was for part of the code that's not something developers do with passion, but rather a suggestion from the Code Quality tool, which ends up in our codebase as a result of copy/paste.

  • 68
    Regardless of what you or I might think, upper management at your company has made it pretty clear that they consider this unprofessional. Assuming you want to continue working there, I think you would be well advised to accept that and adjust your behaviour accordingly. – Steve-O Oct 20 '17 at 20:34
  • 20
    One thing it took me way to long to learn: People perceive the words you use to describe their words, thoughts, actions, or work as describing them. If you say "this code is garbage" then the author of the code usually perceives that as you calling them garbage. What's even worse in this case is that you know the person who wrote that code is able to hear/read your words calling it "garbage" and you also made the statement publicly. Would you go up to a dev in the middle of the bullpen in front of everyone and say, "You're garbage!" Hopefully no. Sadly, that's what you kind of did here. – Todd Wilcox Oct 20 '17 at 21:47
  • 7
    You were probably trying to say "this level of code is beneath you, I know you can do better!" but instead it probably feels like you're saying "you're bad and your work is bad, so you should feel bad". – Cronax Oct 23 '17 at 9:58
  • 15
    As a lead, its beyond unprofessional. Its truly insulting. – Mister Positive Oct 23 '17 at 16:25
  • 11
    "Garbage" is non-constructive. It's as easy an answer as that. – Mike Nov 21 '17 at 22:13

11 Answers 11

96

What would you make of this; do you think my language was unprofessional?

Yes.

Your comment crossed the line from correcting (which typically relies on specific and concrete comments) to insulting ("this is terrible and if we keep it up we're doomed").

It doesn't matter how much experience you have. It doesn't matter if the code is complete garbage. Honesty isn't an excuse for being rude. That is one of the quickest ways to halt a team's progress and momentum. No matter how experienced or productive you are individually, you are going to be hurting your company overall if the attitude you project at the work place hinders your coworkers.

The reason this is particularly troublesome (and why your management likely responded so sternly) is because it actually hurts your team in the long run. You should know that if your goal is to improve the real quality of code produced by your team, then this will actively hurt your goals. The reason why is because when you get a reputation for being critical, and not helpful, people will not come to you for help (nor will they generally listen to you when you try to offer help). As a result any amount of experience you have will be wasted over time, because people will stop listening to you.

Be polite. Offer help and corrections, and not criticism. I personally think that in situations like this an apology (both to the person in question and to the team) is very reasonable.

Edit to add it is certainly good that there was a helpful review in there too, but keep in mind that perception is as important as reality. In many cases it can take just one poorly chosen word to undo any real good that the code review itself does. A very informative code review that ends with (and I understand this isn't exactly what happened) "overall, this is garbage" is still going to leave the coder with a very bad impression overall, and it is quite likely that they will stop listening to everything else you say. It's important to keep the whole thing professional, every step of the way.

  • @Conor Mancone: Please edit your comment per information provided in Update 1/2. – Ostati Oct 20 '17 at 20:39
  • 7
    Perception is reality for everyone else in the room. Once the perception is set the reality is lost. These people are not likely to re-evaluate the situation. – cdkMoose Oct 20 '17 at 21:20
  • What do you think if it was a blind review and no one on the team including the reviewer knew who had written the code except the author? – RibaldEddie Oct 20 '17 at 22:28
  • @RibaldEddie The point (from my perspective) is that one of the biggest issues is the impact on team morale. If it is public (which it is) then everyone sees it, and it colors the way everyone thinks and acts, even if they weren't the code writer. Most people will be afraid of getting the same response themselves, and that's not good for anyone. Not to mention the very real impact it will still have on the person who wrote it. The fact that they don't know who the critical person is will probably make people more nervous, not less. – Conor Mancone Oct 20 '17 at 22:33
  • 7
    "Honesty isn't an excuse for being rude." +1 You can say what you mean in an honest and direct way without being hurtful or derogatory. That's really what this comes down to. – Kat Nov 25 '17 at 21:00
47

It is the nadir of unprofessionalism in Information Technology to be derisive of another's code in such a fashion and it goes beyond the derision to the way you made the point.

  1. It was done publicly, which is why you were pulled aside (virtually)
  2. It disrupts a team.
  3. It breeds resentment.
  4. Coders, like artists take critique VERY personally.
  5. It was a massively demotivating thing to say. Productivity from that coder is going to take hit.
  6. It makes others less willing to take risks, lest they get ridiculed.

I must say in the strongest possible terms that this is a BAD THING. There is no excuse for this behavior. Do not repeat it.

I have my own feelings and quite a few of us out here agree. I can manage someone who has written bad code with a good deal less stress than someone with a bad attitude. I can always help you improve your code, but a person's attitude is beyond my control and I won't deal with someone with a bad attitude.

  • 1
    Great bullet points. I <3 them. – Ostati Oct 20 '17 at 21:01
  • 14
    Re: 4 - IME, everyone takes critique of their work personally. It's common to see one's work as reflecting oneself. – Todd Wilcox Oct 20 '17 at 21:50
  • 2
    It may also cause a slowdown in team productivity due to the team feeling nervous about potential ridicule, so they're less likely to feel comfortable submitting code for review which may actually be fine, for fear of similar treatment. This may lead to more, unnecessary time spent doing self-review, but also with colleagues interrupting each other for peer-review in the long run. – Longisland Oct 23 '17 at 10:00
  • 2
    I think your last paragraph might be a bit much here. The OP reads more like someone who's too direct for his own good or hasn't fully attuned to social norms regarding criticism. That's a corrigible problem and quite different from having a bad attitude. (And of course a manager would still have to deal with it, but you'd handle it differently.) – Lilienthal Oct 23 '17 at 13:05
  • 1
    @GabrielC. No, I said what I meant, but if one word troubles you so much, perhaps you should read the post as a whole. – Retired Codger Feb 14 at 13:47
22
+100

It can be hard for someone offended by language like that to explain their reaction to people who aren't offended by language like that. A rule of thumb that usually works: if the critical statement addresses generalities beyond one specific incident, it's offensive (hence unprofessional).

In other words, the more professional expression would be to stick to your detailed review, which was specific to the particular code under review. You could explain that this bad code is going to improve the code quality score, which means we need to work on the quality metrics too. People are ready to work through and learn from one bad incident. It's all in context.

But when you change to a global context, which is what your one comment did (yep, just one sentence!) now you are criticizing the employees themselves, or the whole company, and that's more threatening. It leads to (as other answers mention) resentment, disconnection, and a loss of collaborative energy.

You can find this difference between global criticism and instance-specific complaint in relationship literature. John Gottman's explanation (see "criticism") is a popular example.

The words you pick do matter, but the context matters more. It's totally okay (in most software shops I know) to say "garbage in, garbage out" because that has a specific meaning and you are using the phrase to describe why a particular design is unsuitable. Go figure.

  • 3
    First sentence is brilliantly put and the rest is all good. Thx. – Ostati Oct 20 '17 at 21:58
10

direct, honest, and straight-forward.

Said otherwise : harsh, abrasive, non-diplomatic.

The tough thing to learn for us programmers is that human beings are not machines. They are not interested in naked truth. Human beings are social animals, and require a good level of diplomacy to be communicated with.

Some even go as far as saying that the real purpose of communication is not to pass information, but to make friendship stronger. I would not go that far, especially in a professional setting, but this is something you should never forget : even in circumstances when nothing is said about you, "direct, honest, and straight-forward" can in fact leave people unhappy with you, which may hinder your work relations later. Ideally, you'd always want to sandwich a layer of criticism between layers of compliment.

  • 14
    You can be direct, honest, and straight-forward without being harsh, abrasive or non-diplomatic. I might even say calling those things harsh, abrasive and non-diplomatic is itself harsh, abrasive and non-diplomatic. – Dukeling Oct 20 '17 at 20:44
8

What is considered appropriate language for a comment would be entirely dependent on your company culture, but simply calling someone's code "garbage" (and the brackets suggest you actually used a different less polite word) certainly isn't very constructive. It isn't going to help anyone improve.

What was wrong with the code? How could they have improved it? What should the developer do differently? That is the only information you should have included. If it was so bad as to go beyond the scope of a comment you should have pulled the developer aside and discussed it in person.

As team leader, and the one with experience, it is incumbent you to help your team improve. That is your job. Belittling them them in a code review comment doesn't accomplish anything good.

  • I described the problem fully so it's not like I showed a one liner review out there.. It was fully documented with suggested corrections throughout. – Ostati Oct 20 '17 at 20:31
  • Constructive feedback was part of the review. – Ostati Oct 20 '17 at 21:02
  • 5
    @Ostati, unfortunately, for most of the people in the room the takeaway will not be the constructive feedback or the fully documented corrections, instead it will be "[garbage] like below". That is human nature. – cdkMoose Oct 20 '17 at 21:23
  • @cdkMoose: I agree but it does not need to be part of the comment, it's better without it is all I meant here. It was said as "code review comment with no constructive feedback".. – Ostati Oct 20 '17 at 21:40
  • 1
    Edited to remove the "no constructive feedback" bit. That was not clear from your original post. The rest of my answer still stands. – Seth R Oct 23 '17 at 12:56
6

What would you make of this; why you think my language was unprofessional?

So ... here's the deal:

  • You are the team lead; that means that you are supposed to build up the team, not beat them down.

  • The code is the developer's work; calling it garbage in writing in front of their peers is a heavy blow.

Okay, that's the part which directly addresses the question. Okay to stop reading now. Next part ... what might you have done differently?

  • Keep the detailed suggestions, those are good

  • In the 'overall' section, say that this file isn't following best practices per detailed notes below

  • Have a breakout, either 1-1 with the guy, or a training session with the team to go through best practices. In other words, turn this mistake into an opportunity

Just my two bits, yo.

  • Your emphasis on the OP's leadership position is critical. I can imagine a situation where it would be appropriate to call a peer's code garbage (if deploying it will be a catastrophe and you need to shock leadership into blocking its deployment), but presumably the team lead is able to block deployment of problematic code. – adam.r Feb 14 at 13:46
2

This is very dependent on company culture. However, there are some general things to consider. There are mainly two reasons why calling code garbage might be considered unprofessional:

  1. It does - by itself - not provide insight as to what is bad or how to improve the situation, it just gives that code a (low) value, while typically you want to look at its advantages and disadvantages rather than consider it generally bad or good
  2. it may be offensive to a colleague, i.e. the person who wrote that code

As for 1), that can be alleviated by providing concrete details as to what you think is bad and how to fix it. Typically, there are reasons why things are as they are and it's not that someone just wanted to write garbage. The code might have been perfect for the job it was intended for when it was written, but the context changed.

As for 2), if the code you refer to is a recent commit from a particular person, that person might feel offended. Especially if he had reasons to write something objectively bad, like time constraints, that you don't take into account. And often what is garbage to you is perfect for someone else, calling it garbage just brings emotions into what otherwise could have been a well founded objective discussion. On the other end of the spectrum - if you take over an old large codebase that no one in the current team already considers "his code", it's pretty impersonal to call that big pile of probably convoluted code garbage (still doesn't address 1. though and you better back it up). Another issue is how you deliver any line containing expletives. The more emotionally loaded (and accusing) the worse, the more light-hearted, perhaps ironically aimed at yourself the more acceptable.

So, if you are unsure, never call something garbage. If you want to test the waters, start using such valuing language for code that no one feels attached to (or your own code) and back yourself up with some details as to what you don't like.

Whether people react offended and consider you (un)professional solely based on the words you use depends on the company culture. However, universally people will judge you based on how you use your words. If you just call stuff garbage without arguing your point of view, without accepting another developers argument as to why the code is fine as it is, people will think you are unreasonable and judgemental. And thus unprofessional. Especially if you explicitly or implicitly blame your co-workers for what you consider bad code. This is true no matter whether you use expletives or not, i.e. saying "this code is just bad", without any further comments, is unprofessional as well, but the more emotionally loaded words you use (and the more emotionally loaded you present them), the worse this gets.

That being said, if you use expletive words in a reasonable fashion without directing them at your co-workers but rather as a means to give weight to your impressions or vent off some frustration, it can be totally fine. I've worked in an environment where the tone was very direct, playful and the sentences often full of expletives. But typically any such expletive was directed at some technical problems, the way some frameworks behaved unpredictably or even at one's own code ("Look at the garbage I did there, it's incredible that I wrote that stuff!"). On the other hand, I've rarely seen a team working so professionally - respecting each other, always looking how to fix the problem and not how to distribute the blame.

TLDR: using expletives to describe code isn't necessarily unprofessional in itself. In some cultural contexts, however, any use of expletives is considered unprofessional/offensive. It is your job to figure out whether this is the case in your environment before using them, not doing this first is unprofessional. If (another person's) code is absolutely valued, using expletives or not, without a) giving details and b) without separating that criticism from the person and c) without knowing that the other person can separate criticism of their code and criticism of themselves, this is unprofessional as well. In your current company, people are obviously not accustomed to any swearing / expletives and consider it bad behaviour - so don't do it.

2

You sent that comment in the wrong direction

When first reading your post I asasumed this was a comment to your management, and my initial reaction was "Well, if they don't like the truth - they should stop forcing your team to use such stupid metrics."

And had this been the actual situation (that the comment was going up the chain) - it would have fine. Possibly a little unprofessional, but realistically the most straight forward way of alerting management that they are screwing your team over.


However, it seems that this wasn't where the comment went. Instead of fighting on behalf of your team against management - to remove stupid metrics that your team agree are causing garbage to be written - you've done the opposite and put your team in a "damned if you do, damned if you don't" situation.

The metrics for code-quality, I doubt were created by your team. They likely already feel these are idiotic hoops they're being forced to jump through.

Telling them that the code they are writing is "garbage"; because of a system they have no control over - is just signalling that you don't have their backs, and don't care to fight their battles.


The unprofessionalism, and where the issue has come from, is not simply the wording of the comment. It's the sentiment that you're going to screw your team both ways.

Rightly, they've stood up to you on this. Their response is effectively "try to blame us for this, and we'll make it extremely clear how this situation is your fault."

The language you used may have given them more of a case to fight you on - but it absolutely isn't the core reason you're getting pushback.

1

Apart from the offensive language, your comment also does hold zero content about what is actually wrong with the code and holds a subtext which opposes your team goals.

If improving the code quality score

So the collective goal is to improve code quality. Why dont you say "We want to improve code quality" then? Why do you take this code-snippet as an argument for not improving code quality.

To me (as a less experienced developer than you), this sounds like a comment from someone who is used to his old ways and does not welcome change (even if its dubbed "code quality").

we have to introduce garbage like below

You might have already collected that calling things other people write "garbage" is not a good way to gain approval, you provide absolutely no information about why anything could be wrong with the "garbage like below" apart from the fact that it rubbed you the wrong way.

A LEADING developer with 15 years experience should be able to provide more information than calling something "garbage".

then we’re in serious trouble.

Why? Why? Why? Use your experience to pinpoint the problem instead of simply stating that there is one. And if you are really that good at your job, point to a better solution, so that your team knows where it is heading.

Sorry, but your comment (extract) really is garbage. By its very definition, it can be tossed into the bin.

Regarding Update 1: If your full comment was detailed and provided all the said information. Why was it necessary to insult the developer (and question your team goals) on top of it?

1

If improving the code quality score means we have to introduce garbage like below, then we’re in serious trouble.

There are certain situations where code can be called "garbage." Such as memory allocation, or unreachable code. However, I don't think you mean it in such a way.

However I think you should use the appropiate terms when referring to certain programming styles. Things I mainly see are race conditions, loop conditional checks recount the array each loop when it doesn't change, etc. I would comment on specifically what is wrong, or if alternative functions can be used to write that down.

  • I love this answer. Garbage actually means something in coding! – user87779 Feb 21 at 0:04
1

Most of the other answers were prior to your edit about this being code suggested by an automatic tool, so let me address that part specifically.

You felt this code was fair game for harsher criticism because it came from a tool, not a human, but you didn't consider:

  • One or more humans made the decision to give this tool a significant role in assessing code quality.
  • A human decided to copy/paste the tool's suggestion instead of addressing the quality issue in a more readable way, either because it didn't occur to them it could be done better, or they didn't know how to do it better.

In other words, there are still human decisions in the loop that are being caught up in the disparagement. I would phrase your objection more like:

I don't think the quality tool's suggestion is very helpful in this particular case, and it frustrates me that the tool is so often misleading like this. It's not obvious, but the underlying issue could be addressed in a more readable way by…

This makes it clear your frustration is with the quality tool and not the code author, and makes your criticism constructive by offering a way to address the issue.

  • Well said. Someone personally has or had a stake in everything at the company. Even some compliance-mandated fix/process/tool that is dictated from outside the company still has someone in the company who--even if they feel bad about it--is forced to make everyone else comply with it. So even under the best of conditions, you're underscoring a sad part of someone's job. – John Spiegel Feb 15 at 19:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.