Don’t Teach during Code Reviews

In this post, I’ll show you a common pitfall you should avoid when you set out to teach during code reviews.

You probably heard it already but, yes, code reviews are an excellent way to teach, learn and help each other.

But, and here comes a big BUT, please do not “play teacher” during code reviews.

So, what do I mean by that?

Well, recently I did an exhaustive search on what people recommend when giving code review feedback for one of my code review workshops. While I saw some excellent pieces of advice, such as here and here, I also came across quite a few examples that are in strong contrast to the experience I have improving and optimizing code review processes at Microsoft and beyond.

Also, the advice strongly contradicts the findings of the latest code review research.

I thought what better way to share my perspective and reveal some code review pitfalls, than using real-world examples? Today’s one is about how (not) to “teach” during code reviews.

Making the Code Submitter Think

I’d like to start with an example from a blog post on Medium*. With over 900 claps on Medium, it seems quite a few people agree with the proposed feedback style. In this post, the author describes how a reviewer should phrase feedback. In this particular case, the reviewer thought that changing the method name to openRequest() would make it clearer what the method does and therefore increase the readability of the code.

But instead of addressing the found issues directly, and telling the code submitter that you suggest a rename of the method, the blog post author suggests writing the following code review feedback.

I’m not sure if I understand the whole idea but could you explain what this method does?

So, why would you write such a vague comment instead of directly saying what you think?

Well, the blog post author’s intention is to let the code submitter reflect and think about the code change on their own.

The author further explains that the first comment should “possibly” make the code submitter think they should rename the function. This would mean the code submitter found the problem alone, and we multiplied the learning effect.

The blog post author further explains that only after the code submitter explained the whole method to the code reviews, the reviewer should write:

“I get it now, sorry for being slow! I’m not sure but do you think we could rename this method to openRequest() or something along those lines?” 

Great Intention – Poor Execution

Well, it seems the post author wants to help the code submitter. What a great intention. Still, I completely disagree with this style of code review feedback, and here is why:

First, I find the fact one person actively makes the other person “think” extremely condescending.

This style of “teaching” shows strong hierarchies and a clear wish for master and apprentice roles.

It’s not bad to “teach” in code reviews, but it should happen on an eye-to-eye level. Also, thinking about learning rather than teaching in code reviews enables a more balanced reviewer-reviewee relationship.

It’s not about who teaches who, it’s about helping each other and learning from each other.

So, a nice way of phrasing this comment is to directly address the problem you see with the code and also explain why you suggest a change.

The why part is where the code submitter can understand your intention and learn from your perspective. In fact, stating what you suggest and explaining your reasons opens up a genuine collaboration opportunity where both parties can learn. Because let’s face it, it’s not always the code submitter that learns. It can very well be the code reviewer that learns something new.

Code Reviews Slow Us Down

Another substantial problem with the proposed feedback loop is that it slows the code review process down.

First of all, based on this vague comment you can only hope that the code submitter “possibly” gets what you are hinting at. Maybe they won’t come to the conclusion that the method name should be changed. Definitely, they will spend valuable time thinking about and trying to distill your message.

Then, they will spend even more time explaining a method to you, that you already understood. Just so you can tell them how you think the code can be improved.

This back and forth takes time. Valuable time. And, I do not see the benefit for either of the people involved.

Also, don’t forget slow code reviews and reduced productivity are one of the major pitfalls during code review. So intentionally dragging this process out is extremely counterproductive.

Code Reviews – An Equal Partner Relationship

Can you imagine how the code submitter feels if they would realize the whole question-answer part was not genuine but constructed by you to make them think?

Well, code reviews can easily become toxic places. And condescending feedback, master-apprentices relationships, and teachers-hats are contributing to such negative environments.

To end with a positive note, I want to wrap this post up with some tips for fast, productive, respectful feedback cycles. But first, let me show you how I’d phrase the feedback for this example:

“I had a hard time grasping what the method does. What about changing the method name to openRequest() to make the methods objective clearer and improve code readability?”

The learning in this type of comment happens implicitly from the explanation you give about why you request that change. In addition, I paid attention to phase the feedback as a question, not a demand, I used I-Messages, and I talked about the code, not the person writing the code.

In general, I’d suggest to

• always stay humble,
• make sure your feedback is genuine and concrete,
• state the why for your particular change request,
• let the code submitted know which solution you have in mind (a sketch might be enough),
• also, keep in mind that the code submitter might come up with a better solution to a problem as s/he is deeper involved in the problem,
• and keep the context and the background of the code submitter in mind. This influences how much detail you put into explaining the “why part” of your feedback and the alternative solutions.

I’m sure this post creates some discussion. Let me know your thoughts and ideas in a respectful and constructive way below!

There is more for you

On Awesomecodereviews.com, you find everything you need to improve your code reviews practice such as code review best practices, code review checklists, latest research on code reviews, and information on how code reviews work at Microsoft, or how Google optimized code reviews for speed.

---

* I did not link to the original blog post on purpose. I also slightly changed the wording to obfuscate the OP, while keeping the exact same meaning. I did so because I do not want to call people out or shame anybody. Still, I wanted to give some context by mentioning the blog post, as it shows this feedback style reflects what quite a few people deem constructive and helpful code review feedback.

Updated on February 7th, 2023