What's That Smell?

We're passionate about writing quality code here at Mesomorphic. Part of creating the best possible code is being aware of code smells.

• 22 April 2017
• By Tim Ash
•  Development

We’re passionate about writing quality code here at Mesomorphic. Part of creating the best possible code is being aware of code smells. You would be ill-advised to ignore the smell of burning when baking a cake or the smell of mould when opening the fridge. These smells are warnings that something isn’t right and that you need to take action. Similarly, a code smell is an indication that something isn’t right with your code. We’ve been using a code smell detector tool for Ruby called Reek over the past few days, and we’re very impressed so far.

Installation couldn’t be simpler: just type

$ gem install reek

and you’re done. Running Reek on your code is just as easy: for example

$ reek MySourceFile.rb

or

$ reek MySourceFileDirectory

to run Reek on a single file or directory, respectively.

Here’s an example of how Reek can be used to eliminate code smells and improve code quality.

Behold some cryptic, smelly code:

class Calculate

  def initialize(x)
    @t = x
  end

  def a(x)
    x * @t
  end

end

Oh dear: obviously this could do with some work. Let’s see what Reek makes of it:

$ reek smell_example.rb
Inspecting 1 file(s):
S

smell_example.rb -- 5 warnings:
  [1]:IrresponsibleModule: Calculate has no descriptive comment [https://github.com/troessner/reek/blob/master/docs/Irresponsible-Module.md]
  [7]:UncommunicativeMethodName: Calculate#a has the name 'a' [https://github.com/troessner/reek/blob/master/docs/Uncommunicative-Method-Name.md]
  [7]:UncommunicativeParameterName: Calculate#a has the parameter name 'x' [https://github.com/troessner/reek/blob/master/docs/Uncommunicative-Parameter-Name.md]
  [3]:UncommunicativeParameterName: Calculate#initialize has the parameter name 'x' [https://github.com/troessner/reek/blob/master/docs/Uncommunicative-Parameter-Name.md]
  [4]:UncommunicativeVariableName: Calculate has the variable name '@t' [https://github.com/troessner/reek/blob/master/docs/Uncommunicative-Variable-Name.md]
$

Reek has produced five smell warnings for our eight lines of code. Each warning has a handy link to a description of why that particular smell may be a problem. Starting at the top of the list, the Irresponsible Module smell is letting us know that this class has no comment describing what it is intended to do. The remaining four smells, Uncommunicative Method Name, Uncommunicative Parameter Name (twice), and Uncommunicative Variable Name, indicate that we could improve our naming system.

Now let’s see how that code could be improved:

#Class to assist in the calculation of Value Added Tax (VAT)
class VATCalculator

  def initialize(vat_rate)
    @vat_rate = vat_rate
  end

  def vat(price_excluding_vat)
    return price_excluding_vat * @vat_rate
  end

end

We’ve added a comment to explain what the class does, renamed the class, methods, and variables. And, although Reek didn’t complain about it, I’ve added some white space. If we run Reek on the refactored code, it doesn’t report any code smells. The intention behind this class should now be a lot clearer - it’s been created to work out the amount of VAT given a VAT rate and a price.

Reek will detect and report on around a couple of dozen code smells, ranging from the prosaically named Instance Variable Assumption to the flamboyantly monikered Prima-Donna Method. A full list of the code smells that Reek will detect, and why they may indicate an issue, can be found in the Reek documentation.

Reek has pointed us to numerous parts of our codebase that we’ve improved and tidied up as a result. However, it’s important not to get too carried away: Reek is just another tool, and just because it reports a code smell doesn’t mean that piece of code would be improved by refactoring. Equally, just because reek doesn’t flag up a code smell, doesn’t mean there aren’t any issues.

Reek: You can read more about code smells here: https://sourcemaking.com/refactoring

Photo: Aqua Mechanical, Utah
Licensed under Creative Commons