How to Do Code Reviews Like a Human
mtlynch.io
mtlynch.io
There's some overlap with the contents of this submission. I'll give some examples:
>"Why did you code it this way?"
(There could be more specific variants: "Why did you make this a class?" "Why did you give it this name?" "Why did you place it in this file?", etc).
This is a poor question to ask. I found I would spend time thinking about it and justifying it, which is draining when asked for every detail in the code.
The underlying issue is this: We are having a conversation amongst equals. The role of a reviewer is not a superior position to the developer. When asked this question, I do not know why it is being asked. If I give a detailed answer, I am putting in a lot of effort whereas he is not, which makes the conversation very asymmetric. He is "taking", but is not "giving".
I recently read the book "Influence" by Cialdini. One lesson I learned is that the notion of reciprocity is deeply ingrained in us. Thus his taking and not giving was, deep down, bothering me (even though I did not know the cause).
Arguably, in this context, you could argue that it should not bother me. However, consider the cognitive load of coming up with all these answers, whereas it is trivial for a reviewer to ask these questions.
The reviewer needs to put in the effort and find flaws - not just sit back and have me explain it all to him.
From now on, I think my response will plainly be "Because it solved the problem".
Alternative question, which is a little better:
>"Why did you use approach X instead of approach Y?"
This a bit better, because at least I now am getting a hint on what his concern is. But it's still making me defensive. I still need to justify it, analyze approach X with approach Y, and think about it.
From now on, my response will be any of:
"I did not consider Y" (if true)
"I did not see any benefits of Y over X" (if true)
"Overall, the pros of Y did not outweigh its cons"
(etc).
Ultimately, the issue is: Reviewer thinks Y is a better way to do it. Instead of saying it outright, he is being very indirect and hinting at it. So I now have to play a game of guessing why he thinks so. He is trying to be clever/polite by thinking up a bunch of questions that will lead me to his conclusion, but almost always those questions get answers he is not expecting, and this gets long winded.
>"Your approach X has these flaws".
Useful if I was not aware of them. If I am aware of them, then, um, so? Am I supposed to respond to that?
For any complex solution, there will always be tradeoffs, and hence there will always be flaws.
I think my standard response to that will be:
"Yes. Yes it does."
>"I see that you used approach X. I think X has the following flaws (lists them out). Have you considered Y, which has these benefits (or absence of flaws)? Would you agree that Y is a better approach?"
This is how people should ask questions in reviews. I don't have to guess what the person is leading up to. No uncertainty in my mind.
I've been reading books about communications in the last year, and they generally say "Ask questions if genuinely curious. Ask questions to verify assumptions/judgments you have made. Do not ask questions to lead the other to a conclusion." (Or rather, state your perspective and then ask questions like the question above).
At a younger age, we were told about the virtues of the Socratic method. It was ingrained in me and even now I involuntarily use the method on others when I'm trying to get them to see something. I can tell you from my experience: It's a bad idea and a habit I need to get away from. I wrote more about it elsewhere and can expand on it if anyone is interested.