I am a junior developer. How do I handle disagreement in a code review regarding an unlikely edge case?

I am working at a robotics startup on a path coverage team and after submitting a pull request, my code gets reviewed.

My teammate, who has been on the team for more than a year, has made some comments to my code that suggest I do a lot more work than I believe to be necessary. No, I am not a lazy developer. I love elegant code that has good comments, variable names, indentation and handles cases properly. However, he has a different type of organization in mind that I don’t agree with.

What do I do? 

I’ll provide an example:

I had spent a day writing test cases for a change to a transition finding algorithm that I made. He had suggested that I handle an obscure case that is extremely unlikely to happen–in fact I’m not sure it is even possible for it to occur. The code that I made already works in all of our original test cases and some new ones that I found. The code that I made already passes our 300+ simulations that are run nightly. However, to handle this obscure case would take me 13 hours that could better be spent trying to improve the performance of the robot. To be clear, the previous algorithm that we had been using up until now also did not handle this obscure case and not once, in the 40k reports that have been generated, has it ever occurred.

We’re a startup and need to develop the product.

I have never had a code review before and I’m not sure if I’m being too argumentative; should I just be quiet and do what he says? I decided to keep my head down and just make the change even though I strongly disagree that it was a good use of time.


I respect my co-worker and I acknowledge him as an intelligent programmer. I just disagree with him on a point and don’t know how to handle disagreement in a code review.

Answer

  1. Unit tests are not just about ensuring that your code is correct now, but also in three years when someone else is maintaining the code you wrote.
  2. It is always a good idea to have more than one reviewer. If your code is only reviewed by one colleague, ask someone else who knows that code, or the codebase in general, to take a look. A second opinion, again, will help you to more easily (dis)agree with the reviewer’s suggestions.
  3. NEVER be angry, ALWAYS speak your mind – with a smile. You should have instantly told the guy, “Hmm, that will take me at least two days, is this worth it, let’s ask our boss?” and secondly you should have said “Don’t forget man we’re working on robotics, it’s actually not possible for the left sensor to be to the right of the right sensor – let’s ask the boss how much anti-physical corner cases we want to cover”.

 

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s