9

As a young guy in my first programming job, I can't figure out how to avoid sounding passive aggressive when providing code review comments or when responding to such comments in my own code. It invariably ends up sounding like "X does Y, not Z. It's not needed here." in response to something like "don't we need to call function X here?".

So how do you properly respond to general review comments like "I think this is missing here" or "I think this is not needed here"? And how do you correct a coworker's misunderstanding without sounding snappy?

I'm asking because a coworker said "I think this comment doesn't accurately describe the code because X actually does Y". I was going to respond with "X actually doesn't do Y. Z is the one that actually does Y". However, it just sounds slightly snarky. I for sure would be a little annoyed if someone responded to me like that.

2
  • Always refer to coding conventions, good practices and be factual. Explain or restate the purpose of the convention/practice if needed. If a workaround from the conventions or the good practices has been used, it has to be supported by a must-have business need validated by the owner. Dec 11, 2018 at 14:02
  • What did you ultimately say if you did not say the "snarky" remark to your co-worker's misunderstanding?
    – Dan
    Dec 11, 2018 at 18:44

6 Answers 6

7

He misunderstood your code.

Ensuring the team understands your code correctly was in order.

Your example states simple facts and would have been just fine as a reply.

The best answer would have been

"Actually Z does Y"

Concise, objective and precise without any personal context.

In professional conversations you stick to objective facts and established procedures.

You leave out personal comments or remarks, pointing fingers or attacking personally.

If someone made an obvious mistake that you're certain of or they misunderstood your own code, you're not only allowed but also obliged to correct them in a respectful manner, avoiding embarrassing or reprimanding them with other people present.

Stating objective facts is not passive aggression and anyone offended by them is unprofessional.

7
  • Bonus points if you point to documentation that proves "Z does Y".
    – jcmack
    Dec 11, 2018 at 0:15
  • So, how would one correct another in a respectful manner? Dec 11, 2018 at 1:46
  • @RichardU as I said, by keeping everything personal out of the conversation and sticking to facts.in OP's example he ought to know his code best.simply stating that he wrote it in a way that Z does Y is more explanation or expression of intent than admonition. Dec 11, 2018 at 1:53
  • A bit off the topic : OP may also address how did the reviewer misinterpretated the purpose of Z maybe because of the lack of comments in-code. If OP gets crushed by a car tomorrow (which I hope won't happen), we wouldn't want the person who replaces him to do the same mis-interpretation mistake. TLDR : Always comment your code self-supportive through comments and namings. Dec 11, 2018 at 14:15
  • @Answers_Seeker actually very important point! Maybe indeed his comments were sub-par or even wrong, leading to the erroneous interpretation... Dec 11, 2018 at 14:21
5

It depends on the environment and on the relationship you have with the colleague whose code you are reviewing (or that is reviewing your code). In some places (countries, teams, workplaces, etc...) being direct, even in code reviews, is not acceptable and considered offensive; in others being too indirect is considered silly. You need to adjust your style to the environment and the people you work with.

For instance, I worked in environments where

Replace X with Y, X doesn't do {whatever}.

was acceptable, and places were

This looks very good to me, but I believe you should use Y here, because X would....

sounded too direct.

I currently work with a colleague who is extremely proud of his work, and that tends to feel diminished even by constructive criticism. I work around this by reviewing his code with him sitting next to me and asking him to describe the code; it becomes a sort of delayed pair-programming session, where we discuss improvements and work them out together. During the process we add notes to the PR, so that he can go back to it later on. We go through a similar process if he reviews a PR of mine and I don't agree with one of his comments.

So, if you are in doubt, you should talk to your colleague, clarify in person and then reply to his comment. If the colleague is remote, you can achieve a similar result with a quick call or a few IMs. If you agreed on the content of your reply, even something like

As discussed, this is not correct, I'm resolving this conversation (To use a GitHub terminology)

won't sound snarky, because of the context.

4

The best way to correct anyone in a situation is by doing so in a manner that allows them to save face, especially when doing reviews like this.

The best way to do it is to do so indirectly.

Hmmmm... Let me just double check... Ah! Here we are. Z does Y. I'm sorry if I didn't comment that clearly enough.

Or something like that. You're right to feel that "X actually doesn't do Y. Z is the one that actually does Y". sounds snarky.

Another way you could phrase that is by gently nudging the person to walk through it again.

Let's check again.... Can you show me what's wrong?

and let the person find the mistake himself.

It's better to let a person save face than to be right.

16
  • 8
    Acting unsure makes it sound like you don't understand your own code. There is nothing snarky about saying"I took care of Y in the Z function". Dec 11, 2018 at 1:24
  • @LaconicDroid no, that way is just a bit jarring. Remember, these are your coworkers and you have to deal with them every day. It's best to let people save face when possible. Be nice, until it's time to not be nice. Dec 11, 2018 at 1:28
  • It's not about saving face (at least in non asian culture) nor about who is right or wrong! it's about what is correct and objective. personal feelings, emotions and booboos have no place in professional discourse.You don't go around insulting/belittling people and they better don't take facts personal.The workplace is not a toddlers playground! Besides, as Laconic Droid said, you don't want to appear incompetent either, especially if it concerns your own work / code. Dec 11, 2018 at 1:31
  • 1
    @DigitalBlade969 You are misrepresenting my point. If the OP wants to come across in a way that is not confrontational, the best way is to present an out, in order to allow his coworker to save face. You never want to put something in a way that will publicly embarrass someone. That is not producive. People are not robots and do not take facts on face value alone. If you don't take personalities into account, you are in for a very rough ride. Dec 11, 2018 at 1:43
  • 2
    @DigitalBlade969 Pon Farr translated into German is "Octoberfest" Dec 11, 2018 at 15:41
3

I would like to add an approach I have found useful - correcting the reviewer's remark concisely, as answered above, but also offering to add a comment, change the documentation or rename the function to make its intent clearer ( if possible ).

Perhaps the reviewer rushed a bit and thus misunderstood your code. But the best code can be read and understood without much thinking, and this code apparently wasn't understood that easily.

Of course, not everything can be made completely simple. But a review comment like that indicates there may be room for improvement on readability here.

Keep in mind you or one of your coworkers may have to understand this code in a situation where there isn't much time or your mind is on other things. Such comments can then prevent misunderstandings and possible mistakes in the future - and just not having to think it through is often a boon in itself.

I've often found myself grateful to encounter a little comment explaining a function call, even in my own code. Things that are obvious when you're deep in the subject matter become less obvious over time, and a couple of words of explanation can save you a lot of frustration later ;)

This also serves to show respect to your coworker, in that it says "Perhaps I didn't make it clear enough", not "You misunderstood this, I'm right".

2

You didn't mention your location, and what you are asking is very location dependent. In the UK, you just state the facts. When you're sure, you say "X does Y, as it is supposed to." When you're not quite sure, you say "I think X does Y, as it is supposed to". There's no "saving face". I'm capable of doing some good work, but I'm also capable of doing outright stupid things, and I also know you are capable of doing outright stupid things, so if you catch me doing something stupid I'll just fix it.

I know there are countries where it doesn't work that way, where it is considered extremely rude to criticise a co-worker (to the point where airplanes have crashed because the co-pilot was too polite to point out the pilot's fatal mistake). I'd love to see a reply from someone working in India, for example.

0

First off, you can't worry too much about how someone else with interpret your comments or infer something from your supposed 'tone'.

I find often that people who accuse someone of 'passive-aggressive' behaviors do so from a defensive position after simple statements of fact cast them in a negative frame, even if the error is very easily recoverable. They just don't want to admit the mistake or oversight. For example:

Statement: The functional guidelines say Z does Y.

Reaction: OMG, why are you accusing me of not reading the spec? The docs aren't very well organized and the part about Z isn't very clear You need to chill out!

In a technical environment, simply stating facts is likely to have by far the broadest appeal and acceptance. If someone gets overly defensive, they have interpersonal issues and you just need to remember that for next time.

I almost universally find that people are reasonable and perfectly fine either accepting the situation or making corrections, even if they're at fault, when the error/oversight/bug is the issue, not the person.

Why doesn't Z do X? That's a really big deal!

Page 4 of Specification 2 say Z does Y, so that's what it does.

Oh, I see. Guess it must have change.

No problem. If Z does need to do X, just update the spec and we'll make it happen.

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .