How do I tell my manager that his code review comment is wrong?

Question

Recently during code review, my direct supervisor wrote a comment suggesting a change on some code I submitted. Normally, that is not a problem (and I understand that is the very point of code reviews), but it turns out that his concern is based on incorrect information.

More specifically, he raised concerns over a certain language feature, and explained why that could be an issue - except that, in fact, the language does not work as he explained. This is supported by language documentation, and is not a formatting/standards issue. (The language issue is about as complex to understand, simple to fix, and makes as much of a difference as a trailing comma in a C array. That is to say, this behavior has never been changed, it is not hard to grasp that the comma is optional, it is not an obscure concept, and it does absolutely nothing. I'm keeping the exact issue a bit vague on purpose.)

My question, then, is how do I explain to him (if at all) that his idea of how the language works is incorrect? Being at least one or two decades junior to him, I don't want to come off as arrogant. I would normally have no problem with sucking it up and making the change (it's as easy as deleting a comma) - but I also feel like that goes against the spirit of a code review.

Not a duplicate of How do I tell a coworker he's wrong?, since this is specifically about telling your seniors they are wrong..

Answer 1

Be humble.

I'm a bit confused here. My understanding was that feature X has property Y. Is there something I'm missing here which means this doesn't work in this case?

Much better to start from the assumption that you are the one that is wrong, rather than the other way round. If you're wrong, you'll learn something; if you were in fact right, you've now taught your manager something in a non-confrontational way.

As an important aside, while you say this is different from our other question, it isn't really: you should be humble with your peers and with your juniors as well.

Answer 2

You can simply "ask for more clarity" and show willingness to change but subtly indicating that his concerns are not valid. An email to this effect may help:

You have requested following changes in the code because of so and so reasons. However, my understanding of this language is that this problem will not occur because it instead works this way. I suppose I am missing something but could you please clarify or confirm that my understanding is wrong? If you have few minutes, I can show a demo of this and may be you can point out more specifically what your concerns are.

This way you will at least have a documented proof that you tried to correct him back. If he still insist, you can do whatever he requested and just hope it wont be a problem in the long run.

Answer 3

In one of your comment you say:

"it is trivial to conclude that the requested change has no effect"

Based on this, the easiest way forwards would be to keep the change and spend your energy on something more important.

Obviously this is only applicable if the change is indeed harmless.

Answer 4

If your manager gets this part of the language wrong, is it possible that other developers will also be confused by it? Bear in mind that maintainability by other people should be a goal. So: if this feature is confusing, there's an argument to be made for not using it.

With that in mind, you can go to your manager and -- being humble -- state that you were surprised by his comment, and so you double-checked the documentation. It turns out that this feature is as you described, but you accept that it could be confusing, so you agree that you'll adopt his suggested clearer way to write the code.

Answer 5

I feel that Philip Kendall answer is incomplete and lacking some backup so that is why I decided to write my own.

I often struggle to make people understand that they said something false but here are some techniques that often works for me:

Technique One

If someone makes an unsupported claim that I'm afraid is false, I ask them for backup. Like:

Oh, this is interesting/disturbing/curious, do you have any source for that? I want to know more about it.

Then, I let them search and they will, hopefully, realize their mistake on their own. Since they are the one who realizes their mistake, they won't feel the shame that often comes with the fact of being corrected.

If this doesn't work, I use technique two.

Technique Two

If the previous technique didn't work or isn't possible, I act confused and ask for clarification. This way, I'm showing that I'm open to the possibility of being wrong myself and that being wrong isn't such a big deal.

I usually start with some basic questions and then ask the more complex ones, like:

Me: So, <some basic question>, right?

Other: Yes

Me: And, <some other basic question>, right?

Other: Yes

Me: So, knowing that, how could <your real question>?

Using this series of question allows you to be sure that you are on the same ground knowledge and that you don't have some misunderstanding somewhere.

When they answer your last question, if you are still not satisfied with their answer, you can ask follow up questions, like:

I still don't understand. When I read X, I understand Y. Isn't that right?

You notice that, during the conversation, I keep blaming myself for the misunderstanding. By doing so, you are not pressuring the other to be right. Since you can acknowledge being wrong with no shame, they will be more at ease to do the same.

---

I use this technique often and, so far, it's the best I have. Note that, in my experience, heated arguments are really bad and should be avoided at all cost.

Answer 6

I don't see the need to be so indirect and "humble" about this.

You can make it pretty polite and simple and clear without coming across as arrogant:

Manager:

If you overflow an integer in C, it's undefined behavior, so add a check for that.

You:

I think unsigned overflow is guaranteed to wrap around in C? Only signed overflow is left undefined. (I see it here.)

This seems just fine as a first response to me. The only way I could see this coming off as arrogant is if you have a back-and-forth argument.

Answer 7

I suggest a matter-of-fact Socratic approach that avoids the need to be either humble or accusatory:

Hi NameOfBoss, about line X in function Y; I believe it might benefit from some additional clarification. Mind if we take a quick look?

This comment states A. Meanwhile, this documentation here states B. On face value, these two statements seem to contradict each other. If this is intentional, I believe it would be prudent to document why A is correct in spite of B. As the author of comment A, can you help me with that?

Answer 8

I would handle it in two ways, depending on the supervisor. If the supervisor is a tech person that works in the language you code in - I would contact them through an email with something along the lines of:

During the last code review, you have commented part xyz of my code as invalid and it got me wondering if my knowledge of how it works was correct. I have checked against the documentation and made a small demo application and it seems to work correctly - should I still make the change or keep it in the current version?

Do include links to the documentation about that code part and a small demo - most developers, both junior and senior, are eager to learn new things in the field they work in and there is always a chance that it was just an honest mistake of your supervisor. It also shows that you make sure the knowledge you use at work is correct, which is usually well received by supervisors, while not attacking the supervisor in any way.

If your supervisor does not work with the language you use (as some teams often use many different technologies and the supervisor has to learn them a little for the sake of code review), and as you mentioned it is not a big change, I would change the code and not address it further - trying to persuade that your version is correct and their knowledge is too small would probably create more tension and negative feelings between you and your supervisor, than it would create good language practices.

Answer 9

"Thanks for the feedback. After additional review, I see conflict X in your recommendation, please help me understand if this conflict is not the case or I have misunderstood your feedback."

Answer 10

As you are saying that do the change is much easier, First you do the change and get it reviewed once again. At time of submitting the changed code for review once again you can simply ask that

Hello Jim, I checked the documentation provided for language and my understanding of the feature X working in language is as like this.

You can then provide a link to the documentation and ask him if what you understood from document is correct or not.

Answer 11

In reading some of these workplace problems, it brings back old memories where I worked in a SLC, UTAH company, in a manufacturing environment as a programmer. In looking back on it, I burned a bridge which I should of NEVER let happen. It was a bridge that I had to cross many times before. My supervisor was known as the company drunk, but he knew his stuff. He had many problems, but that side of him, was none of my business in the first place. But NEVER tell your immdiate supervisor that he/she is wrong. NEVER! And never in front of his peers or any of his/her supvervisors. That a strike or nail in your coffin. But before you runoff at the mouth on him/she being wrong, first take some extra time plus some extra effort(s) on your part and research it all the problems out where he/she 'might' be incorrect in full and provide alternate answers or solutions to solve the problem(s). Notice the careful wordings in the above statement??? Use words like "'might' be incorrect" NEVER 'Wrong'. Have supporting documents and answers to support your theories. After you've done your extra homework, set up a time to meet with your supervisor in a meeting form so that it fits his/hers' schedule, not yours! And go into that meeting with an attitude showing that you are workable, friendly, and happy and SHOW IT! One thing I learned with this past supervisor, he could read my attitude and body language and facial features and voice - know everything about me. And with all this effort and a meeting, you could either loose or win. As my father would say, your supervisor holds the future of your job within the company you work in. In a sense, you are at his/hers' mercy. Oh, one thing I forgot, DON'T raise for voice for anything! Again, as I look back on this job, I could see places where the bridge was starting to burn. My job as a manufacturing programmer, is just like yours. All my work was reviewed by my supervisor period! And I recieved good marks and bad marks on my work and sometimes I was told to redo my programming work, and it was humbling! So my advice to you as a from manufacturing environment programmer, It's up to you if you want the job, if you like the job and want to stay - it's up to you and your attitude plays a big part in your job. Remember, you are to make your supervisor look GOOD to everyone!! Enough of my preaching... Now enjoy your job.