4 ms·

Agree with all of this except 4. "asking judgmental questions". While it's true that not all opinion is fact (uh oh -- 5. sarcasm) there is nothing out of line

Agree with all of this except 4. "asking judgmental questions". While it's true that not all opinion is fact (uh oh -- 5. sarcasm) there is nothing out of line in asking "Why didn’t you just do ___ here?" How you approach this question has a lot to do with your own perspective. If you are feeling defensive, it can seem like the code reviewer is calling you out. But if you have experienced people reviewing your code, this is a question that you will get a lot. "Why didn’t you just do ___ here?" often simply means "I don't see why you couldn't have done it this simpler way, can you explain"? It's often for everyone's benefit that things are done in the simplest, most obvious way possible, and it's not productive to take offense at being asked to reconsider how you accomplished something.

rpedela 8y ago
I think OP's point is that the same question could be asked in a less confrontational way.
- deleted 8y ago
  [deleted]
jaggederest 8y ago
I think that's a perfect example of what the article is about. Instead of "why didn't you do X here?" You can say "I would prefer X here, because I value Y" and then add your reasoning. It's a more complete statement and it avoids an extra back and forth for someone to say "What do you mean?", which is usually the response to "Why didn't you X?" And it still accomplishes the same goal of opening a conversation, while better making clear you are seeking understanding. It's also more useful to people who might refer back to the PR later, which happens surprisingly often in my experience. I try to treat PRs a little bit like historical documentation.
- edmundhuber 8y ago
  I think the suggestions in the article are the victim of a lack of context, in a certain way. Suppose the code was "list.extend([1])" (Python). And the comment was "why didn't you just list.append(1)"? That would be a completely reasonable thing to say. I think what I want to caution is that there is a middle ground that this article is ignoring. Yes, sometimes it is uncouth to just say "why didn't you just _". But it can also be important to have a little bit thicker skin, as sometimes you will get a comment that looks just like that, and there will be a good reason.
- deleted 8y ago
  [deleted]
erikerikson 8y ago
I've found it helpful to lead with my explicitly labeled expectation I had that the code doesn't express and the reasoning I had for the expectation followed by a request for context. Sometimes better factoring wasn't top priority in the set of many concerns being solved for and sometimes a technique was not known. Oftentimes my expectation wasn't talking interactions in the code into account and the education I receive helps me better review the code or better understand the codebase. I mean, we could deal with the noise of conflict or we can more efficient solve the problem that motivates our effort in the first place.
pa7ch 8y ago
I often ask questions like that because I genuinely assume there is a good reason something was done a certain way but its not obvious to me. Maybe they made it over complicated by accident or, more likely, since I didn't spend as much time thinking about this code I just missed something during review. Maybe there is a way that can better convey that sort of genuine curiosity.
ryanobjc 8y ago
The problem with the question form "why didnt you just do ___ here?" is it's loaded with the assumption that the other party should have known to do this other thing instead. A better approach is to use the less loaded "have you considered doing X here?". If you find yourself needing to demand others not take offense at your questions, you might wish to try a different approach.
- detaro 8y ago
  And if you get something along the lines of "of course I have considered X!" as a response, someone took that one as the loaded variant. It's tricky. "What am I missing?" or a variation on it, as the parent mentions as the intended meaning, probably isn't a bad thing to add, since it makes it clear that you aren't accusing.
- carussell 8y ago
  A "Have you considered..." question asks for a "yes" or "no". A "Why[...]" question asks for why. Assuming that "Why didn't you just do X?" isn't really just a disingenuous way to say, "you're an idiot; do it this way instead", then you should ask for what you want and ask the why question, not the yes-or-no question. In the case that it is a disingenuous question, then that's something that's covered adequately by #5. Suggesting the avoidance of sarcasm is already broadly applicable enough to cover using questions when you really mean to make a statement. (Which is not specific to code reviews—it's as obnoxious in real life.)
- Izkata 8y ago
  I've used "why didn't you just ____" version plenty of times, and in a handful of them, they replied with a paragraph or two outlining the exact reasons - including a very edge-case issue that I'd not noticed. After that I usually reply with something like "Could you add a comment and/or test highlighting this reason? (So the next time someone modifies this code they don't make the same mistake I did.)" Definitely better results all around.

rpedela 8y ago

I think OP's point is that the same question could be asked in a less confrontational way.

deleted 8y ago
[deleted]

jaggederest 8y ago

I think that's a perfect example of what the article is about. Instead of "why didn't you do X here?" You can say "I would prefer X here, because I value Y" and then add your reasoning. It's a more complete statement and it avoids an extra back and forth for someone to say "What do you mean?", which is usually the response to "Why didn't you X?" And it still accomplishes the same goal of opening a conversation, while better making clear you are seeking understanding. It's also more useful to people who might refer back to the PR later, which happens surprisingly often in my experience. I try to treat PRs a little bit like historical documentation.

edmundhuber 8y ago
I think the suggestions in the article are the victim of a lack of context, in a certain way. Suppose the code was "list.extend([1])" (Python). And the comment was "why didn't you just list.append(1)"? That would be a completely reasonable thing to say. I think what I want to caution is that there is a middle ground that this article is ignoring. Yes, sometimes it is uncouth to just say "why didn't you just _". But it can also be important to have a little bit thicker skin, as sometimes you will get a comment that looks just like that, and there will be a good reason.
deleted 8y ago
[deleted]

erikerikson 8y ago

I've found it helpful to lead with my explicitly labeled expectation I had that the code doesn't express and the reasoning I had for the expectation followed by a request for context. Sometimes better factoring wasn't top priority in the set of many concerns being solved for and sometimes a technique was not known. Oftentimes my expectation wasn't talking interactions in the code into account and the education I receive helps me better review the code or better understand the codebase. I mean, we could deal with the noise of conflict or we can more efficient solve the problem that motivates our effort in the first place.

pa7ch 8y ago

I often ask questions like that because I genuinely assume there is a good reason something was done a certain way but its not obvious to me. Maybe they made it over complicated by accident or, more likely, since I didn't spend as much time thinking about this code I just missed something during review. Maybe there is a way that can better convey that sort of genuine curiosity.

ryanobjc 8y ago

The problem with the question form "why didnt you just do ___ here?" is it's loaded with the assumption that the other party should have known to do this other thing instead. A better approach is to use the less loaded "have you considered doing X here?". If you find yourself needing to demand others not take offense at your questions, you might wish to try a different approach.

detaro 8y ago
And if you get something along the lines of "of course I have considered X!" as a response, someone took that one as the loaded variant. It's tricky. "What am I missing?" or a variation on it, as the parent mentions as the intended meaning, probably isn't a bad thing to add, since it makes it clear that you aren't accusing.
carussell 8y ago
A "Have you considered..." question asks for a "yes" or "no". A "Why[...]" question asks for why. Assuming that "Why didn't you just do X?" isn't really just a disingenuous way to say, "you're an idiot; do it this way instead", then you should ask for what you want and ask the why question, not the yes-or-no question. In the case that it is a disingenuous question, then that's something that's covered adequately by #5. Suggesting the avoidance of sarcasm is already broadly applicable enough to cover using questions when you really mean to make a statement. (Which is not specific to code reviews—it's as obnoxious in real life.)
Izkata 8y ago
I've used "why didn't you just ____" version plenty of times, and in a handful of them, they replied with a paragraph or two outlining the exact reasons - including a very edge-case issue that I'd not noticed. After that I usually reply with something like "Could you add a comment and/or test highlighting this reason? (So the next time someone modifies this code they don't make the same mistake I did.)" Definitely better results all around.