Skip to content

Instantly share code, notes, and snippets.

@drbrain
Last active August 29, 2015 14:19
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save drbrain/5ea7c9389a55c81fd3c1 to your computer and use it in GitHub Desktop.
Save drbrain/5ea7c9389a55c81fd3c1 to your computer and use it in GitHub Desktop.
Parentheses don't make code readable by themselves

I've heard people argue "parentheses make code more readable" or "parentheses make code less ambiguous". Parentheses neither help nor hinder ambiguity or readability, your style decisions beyond use of () do that.

Here's a bug we introduced in our application that was allowed through by a coding style that preferred parentheses. If you follow a coding style that omits all unneccesary parentheses (only those that suppress warnings, which I prefer) this bug would have been impossible.

The bug is simple but subtle. The environment-specific data wasn't fetched from config/two.yml because a ) was in the wrong place. Neither my coworker who wrote it, another coworker who reviewed it, nor my own review of the line caught it.

Without parentheses, though, there is no way to introduce this class of bugs because you can't chain off of intermediate results without introducing a local variable.

The main difference between the two examples is not the use of parentheses, it's the style in which the code is written. When you can't use parentheses you're forced to separate work into separate lines. You're forced to use a local variable instead of chaining another method off the return value.

If you've already adopted a style where you seprate work the same way you would in a parentheses-free style, what do the parentheses add beyond line noise? When I read code that requires parentheses I eventually stop paying attention to them and treat them like whitespace. Then I let bugs slip through during code review even when I carefully read the changed lines.

config = YAML.load_file(full_path_for('config/one.yml'))[environment]
config.merge!(YAML.load_file(full_path_for('config/two.yml')))[environment]
one_yaml = YAML.load_file full_path_for 'config/one.yml'
two_yaml = YAML.load_file full_path_for 'config/two.yml'
config = one_yaml[environment]
config.merge! two_yaml[environment]
@geeksam
Copy link

geeksam commented Apr 17, 2015

I'm not sure it's completely fair to blame the parentheses for this; the original version was just doing too damn much. When I write code like that, I tend to refactor it to the second version (plus parentheses, since I follow a Weirich-style rule for those) purely in self-defense so I don't have to think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment