Code Review Etiquette

by Troy Grosfield
December 9th, 2013

I always enjoy code reviews because they usually bring up some great teaching points and discussions. With that being said, I always feel compelled to make sure everyone’s on the same page with code reviews.

Don’t Take it Personal!

First and foremost, don’t take comments personal! Developers are short and to the point with comments and usually don’t sugar coat things in reviews. This can make comments seem more harsh than they were meant. The important point to take away is the message behind the comment and not necessarily the delivery of the message.

Comment With Facts, Not Theories

Early in developers careers, I’ve seen comments that are merely theoretical and have not merit to them. When making a comment either cite your source, or at least be prepared to backup your comment with a link pointing to the a source of your information. I’m not necessarily talking about either. I’m talking about websites dedicated to standards. For example, if you’re in a discussion about code formatting in Python, follow pep8 standards:

This keeps everyone on the same page for what generally accepted across the developer community.

Provide Examples

If I see a piece of code as follows:

class SomeObj(object):
    some_attr = {}

and see a comment that says:

This code will produce an error.

Some may be left hanging.

  • Why does this produce an error?
  • Under what circumstances?
  • What are some suggestions for ways to improve the code?

Instead, make this a point of emphisis to teach others. A good comment may be constructed as follows:

Instantiation of this class will present problems when initializing 
a mutable object: 

  >>> class SomeObj(object):
  ...     this_is_bad = {'hello': 'world'}
  >>> a = SomeObj()
  >>> a.this_is_bad
  {'hello': 'world'}
  >>> b = SomeObj()
  >>> b.this_is_bad
  {'hello': 'world'}
  >>> b.this_is_bad['my_messy_change_that_affects_a'] = 'oh crap'
  >>> b.this_is_bad
  {'hello': 'world', 'my_messy_change_that_affects_a': 'oh crap'}
  >>> a.this_is_bad # <-- didn't expect this change
  {'hello': 'world', 'my_messy_change_that_affects_a': 'oh crap'}

Instead try:

  class SomeObj(object):
      this_is_good = None

      def __init__(self, this_is_good=None, **kwargs):
          self.this_is_good = this_is_good or {'hello': 'world'}

This explains why it can produce errors or unwanted side affects, show code samples reproducing the errors and provides an an alternative approach.

How Will Your Comment Be Received?

Even though the reviewee should understand not to take your comment in a harsh manner, make sure you are aware of how you are delivering your message. Making negative comments such as:

  • "This code is crap"
  • "I've seen better than this"
  • "You're better than that"

can hinder one's desire to even participate in a code review. Remember, not everyone may be as smart as you in a review. Instead, a few minor word changes can make a huge difference:

  • "This code can produce errors. Let me show you how..."
  • "An approach should be followed similar to what we did with xyz. Here's the link..."
  • "Code this similar to how you did you last project where you..."

The truth of the matter is, it's sometimes hard not to get protective of your code. Make sure you act as a teacher or mentor and do your best to try and not be insensitive or offensive.

No Comments »

No Comments Yet

Leave a reply