I'll reply to your comment and simultaneously address a lot of the subcomments.
Most of what I say is based on what I've read in communications books. One day I hope to write a long post or a series of posts on the (mostly common) advice in the various books I've read.
Disclaimer: I'm not good at these communications tactics live, and often don't put the effort in while posting online, so don't use my comment history as a judge of my knowledge ;-)
> While I agree with the core of "DON’T: Criticize the person. ", I'm not sure I agree with the example.
The example is "Why are you using this approach? You’re adding unnecessary complexity."
To which a perfectly valid response is "Because it solves the problem, and I disagree that the complexity is unnecessary." Not a very constructive conversation, but completely valid. If you're not going to do the work to explain your concerns, I'm not going to do the work to satisfy you. A code review is not an arena where the reviewers get to ask and you the author are compelled to answer every concern. It is a dialogue, and a 2 way street.
I do agree with you and I don't like their suggestion either: "This concurrency model appears to be adding complexity to the system without any visible performance benefit."
Well, umm, that's just your opinion, dude. What do you want me to do about it?
The reviewer did not specify what/where the complexity is, nor an alternative. And worst of all, (s)he is not even asking a question. As the reader, I don't feel compelled to respond. If the reviewer has a better approach handy, by all means suggest it and point out its pros/cons, etc.
Regarding "you" statements, there is almost always a way to phrase them as "I" statements, but the important catch is that when you do, you usually should request something. The example in the blog post didn't, and it very much comes off as someone's opinion. What am I supposed to do with it?
There's no reason not to be direct. But there are different ways of being direct, and you'll be a lot more successful if you give your perspective.
nerdponx said:
> "Why use XYZ pattern instead of ABC? This seems more complicated than necessary right now."
At least this asks a question and gives an alternative. It still lacks any substance on why the reviewer thinks it is complicated (although in some contexts the difference in complexity between XYZ and ABC may be obvious).
Also, in my experience, statements like "complicated", "complex" are often tactics to simply force the reviewer's preferred coding style, and this is a lousy tactic. If I were the author I would ask them to clarify why they think it is complicated. If I were to think of the worst code review experiences I've had, it's where the reviewer is simply pushing for his/her preferred style. Some people like continue statements in for loops, others prefer heavily nested loops. Neither is better. Either have a policy about them in the team or don't highlight it. I don't want to keep switching styles back and forth depending on who will review my code.
> There's also a useful communication/pedagogy technique I learned many years ago called "praise, correct, praise".
Agree with other commenters that this can explode in your face. It is also a tactic that is recommended against in some of the books I've read. In the long run you also lose credibility where people don't take your legitimate praises seriously, and get defensive any time you praise them.
The tactic that is recommended is "Agree, and disagree." State first on the things you agree with the other person on, and then say "And I also think ..." and state your disagreement.
And obviously, don't say "But..."
> Oftentimes I’ll say “It seems like <bad thing>, I would probably <different approach>.”
This is OK if you are just expressing an opinion and don't require the author to respond or to change it. If, OTOH, you really want this changed, then this is a poor way of pointing it out. As a reader, all I see is an opinion. We all have them, and differences in opinion are fine. A difference in opinion alone is not an obligation to discuss it.
> "Can you walk me through the rationale for this concurrency model? It seems complex for what we're trying to achieve at this stage."
I understand that you're seeing it as complex, but I did it this way because it solved the problem and I did not find it as complex. If you have an alternative solution that solves the problem that you think is not as complex, feel free to suggest it.
I think asking for a rationale without suggesting an alternative means the usual (truthful and valid) response is "Because it solved the problem." There are many scenarios: Maybe I couldn't think of any other approach. Maybe I thought of 5 of them and had reasons to select this one. Do you want me to list a lengthy response examining each of the approaches I thought off with a justification on why I didn't select it? I shouldn't have to do that much work.
It's much easier to respond if you proposed something and I'll address that. My response may well be "Thanks - I hadn't thought of that approach and agree with you."
As a general rule, do not ask questions that could require a lengthy answer unless you are willing to offer something on your side. It requires almost no effort on your part to ask a question, but it always requires effort - sometimes a lot - for the other person to formulate a non-dismissive response. Don't make the other person work hard without good reason. If you're going to ask a question, put in the effort to explain your perspective. Never, ever, assume that the author has an obligation to respond to you.
The Socratic approach, frankly, is a poor approach. It works in a very limited arena (e.g. teacher/student), and code reviews are not part of that arena. I learned this the hard way over many years where I'd ask very innocent questions, and occasionally someone would snap/shout at me. Only when I read the books did I understand: When asking a question, you have to make your concerns clear at the outset. You have to make the person understand why answering is a good idea. And you should never feel entitled to a response (and your words/tone/posture has to match).
Finally, as a general rule, be humble as a reviewer. Very often, the author knows more about the code or domain (or both) than you. And very often the author will have already considered your approach and has a very good reason not to go with it. It's perfectly fair game to point out that if the author had such a reason it would be good to put a comment about it as other people reading the code will share your concerns. But acting as if you know better is already sabotaging the conversation.
I would respond with "I do not understand the rationale behind the approach you picked. In my opinion approach XYZ is preferred and here are the benefits (list them all, including complexity). Did you happen to consider XYZ? If you did I'd like to hear your thoughts on the pros/cons of your approach over XYZ."
Paranoid people may read more in my response than there is, but you can't control that too much.