Old Emmanuel Oga's Weblog (new one is at www.emmanueloga.com)

Have you ever been told….

Posted in Uncategorized by emmanueloga on diciembre 28, 2008

…that your code sucks, that one of your ideas is horrible, totally unusable or even (alas) that you or other developers are dumb because they choose to do things in certain way? I have, once or twice…. :-).

Leaving aside the fact that the person proffering those kind of comments has probably not reached maturity yet and/or has not good people skills, there are some good tips to take into consideration when that happens.

I’ve stumbled upon this excellent article:  http://mumak.net/stuff/your-code-sucks.html. I will simply copy-paste cites from it in blue. It talks specifically about code reviews, but we can adapt its tips if we think of a comment like mocking/stubbing is a horrible idea and it can hurt the development process” as if it were a code review… (By the way, David show us in his article the proper way to handle these kind of statements. Excellent post!)

First, don’t take it personally. Very often programmers tend to confuse personal preference with objective worth. I tend to take the “let’s do it your way” path -in cases where there is no harm in doing so- to avoid never ending discussions.

In the positive side of things, consider:

…someone has just tried to improve your product. They’ve put thought, effort and creativity into helping you, and now they have put their work up for critique: thank them.

Divmod have a policy of always saying one good thing in each code review. There is always something nice to say, even if it’s just I’m glad someone is looking at this part of the code

Code reviews provide an amazing opportunity to grow as a programmer and to improve the software we make. Off course! Just because the comment does not come wrapped in a polite sentence does not mean that it is wrong.

At the other side of the coin, If we are the ones reviewing other people work, then:

Ad hominem

This ought to go without saying: review the code and not the coder. Comments about a person will only make it harder for that person to apply critiques about their code.

When making negative comments, refer to the patch or the branch rather than you. For example, You’ve introduced a bug in get_message becomes this patch introduces a bug in get_message.

Unclear Outcomes

If all you say is, this patch introduces a bug in get_message, then you have failed as a reviewer. The goal is to improve the code, not to provide a series of puzzles for the author.

The author should be able to look at a review and be able to tell how to address each point and also when they have addressed all points. Reviews with unclear outcomes turn into open-ended discussions about the patch, which sometimes become focused on making the reviewer happy, rather than improving the code.

Confusing personal preference with objective worth

This is a problem in all spheres of review. Film critics, literary editors and acadamic reviewers all do it. What I like is not necessarily the same as what’s good, although part of becoming a better programmer is having your preferences align better with reality. What I dislike is perhaps even less likely to be the same as what’s bad.

When reviewing a perfectly acceptable patch that solves a problem using imperative-style programming, do not criticize it simply because it isn’t in a more functional style. Doing otherwise makes reviews a game of guess what the reviewer likes rather than write good code.

Reviewers can avoid this trap by phrasing review comments as questions, Did you consider using a more functional style?, Why aren’t you using regular expressions to solve this problem? etc.

I encourage you to read Jonathan Lange’s post, there are a lot of excellent tips on it!

Tagged with: ,

Una respuesta

Subscribe to comments with RSS.

  1. ViR said, on enero 14, 2009 at 4:03 pm

    Excelente post! Es necesario ser un buen comunicador para ser buen coequiper.



Introduce tus datos o haz clic en un icono para iniciar sesión:

Logo de WordPress.com

Estás comentando usando tu cuenta de WordPress.com. Cerrar sesión /  Cambiar )

Google+ photo

Estás comentando usando tu cuenta de Google+. Cerrar sesión /  Cambiar )

Imagen de Twitter

Estás comentando usando tu cuenta de Twitter. Cerrar sesión /  Cambiar )

Foto de Facebook

Estás comentando usando tu cuenta de Facebook. Cerrar sesión /  Cambiar )


Conectando a %s

A %d blogueros les gusta esto: