Reek vs RuboCop

Or code linters / analysers for Ruby

I initially stumbled upon RuboCop, a code quality analyzer for Ruby by chance. My interest was immediately piqued.

After trying it out but before committing to use it I searched for alternatives and found Reek, a code smell detector for Ruby.

Both project analyse Ruby source code and give you a series of warnings. Both can integrate with Rake and both have plugins or third parties that allow them to integrate with editors like Vim.

My first instinct was to search for “reek vs rubocop”, a search that led to no results. I was expecting to see some blog posts comparing the two.

I then proceeded to try them both on the same set of files. I then realised they are different programs that can complement each other rather then replace each other. While they will sometimes overlap they each have a different focus.

RuboCop

RuboCop’s main focus is on coding style and adherence to the Ruby Style Guide. Typical warnings will be about:

  • lines being too long
  • methods having too many lines
  • following Ruby idioms prescribed by the guide, like which types of quote to use or preferring each over for for iteration.

Feature wise RuboCop has more features. It has many output formatters and more options to work with. The output is also in color which is nice. Oh and I really like the name, seriously 🙂

Reek

Reek for it’s part will focus on code smells that could lead to refactorings and that aren’t style related.

Examples include:

  • methods with too many parameters
  • code repetition that could be extracted in a new method
  • unused parameters

Like RuboCop, Reek can be configured via a config file, but overall RuboCop has a bigger feature set (which may or may not be a good thing).

While RuboCop will help you enforce style guidelines on your project, Reek will allow you to find bad code that can be made better through refactoring.

Overlap

Both will give out a warning if a class has no class level comment. Both will also give a warning if there are too many lines/statements in a method.

RuboCop will count the number of lines in a method and warn if that number is over 10, Reek on the other hand will count the number of statements in a method.

There are also other overlaps.

They will usually give you a varying number of warnings for the same file. This has a tendency to go over both ways but in the end which one will give you the most warnings will depend on your coding style.

Example

Here is the default output for both linters on the following file:

class Particle
  attr_reader :stability_radius

  def initialize(stability_radius=1)
    @stability_radius = stability_radius
  end

  def drop(height_map, x, y, size_x)
    drop_point = x + y * size_x

    if height_map[drop_point] == 0
      height_map[drop_point] += 1
    else
      agitate(height_map, x, y, size_x, drop_point)
    end
  end

  private

  def agitate(height_map, x, y, size_x, drop_point)
    # compile list of lower level neighbors
    lower_neighbors = []

    for index_x in x - @stability_radius..x + @stability_radius
      for index_y in y - @stability_radius..y + @stability_radius
        check_and_add_neighbor(index_x, index_y, size_x, lower_neighbors, drop_point, height_map)
      end
    end

    if (lower_neighbors.length == 0) then
      # no lower neighbors, leave particle on drop point
      height_map[drop_point] += 1
    else
      # randomly select one of these neighbors and put particle to this neighbor
      selected = lower_neighbors.shuffle.first
      height_map[selected] += 1
    end
  end

  def check_and_add_neighbor(x, y, size_x, lower_neighbors, drop_point, height_map)
    unless height_map[x + y * size_x].nil?
      if height_map[x + y * size_x] < height_map[drop_point]
        lower_neighbors.push(x + y * size_x)
      end
    end
  end
end

Running:

rubocop particle.rb

Will produce the following output:

Inspecting 1 file
C

Offences:

particle.rb:1:1: C: Missing top-level class documentation comment.
class Particle
^^^^^
particle.rb:4:34: C: Surrounding space missing in default value assignment.
def initialize(stability_radius=1)
^
particle.rb:20:3: C: Method has too many lines. [12/10]
def agitate(height_map, x, y, size_x, drop_point)
^^^
particle.rb:24:5: C: Prefer *each* over *for*.
for index_x in x - @stability_radius..x + @stability_radius
^^^
particle.rb:25:7: C: Prefer *each* over *for*.
for index_y in y - @stability_radius..y + @stability_radius
^^^
particle.rb:26:80: C: Line is too long. [97/79]
check_and_add_neighbor(index_x, index_y, size_x, lower_neighbors, drop_point, height_map)
^^^^^^^^^^^^^^^^^^
particle.rb:30:5: C: Never use then for multi-line if/unless.
if (lower_neighbors.length == 0) then
^^^
particle.rb:30:8: C: Don't use parentheses around the condition of an if/unless/while/until
if (lower_neighbors.length == 0) then
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
particle.rb:34:80: C: Line is too long. [80/79]
# randomly select one of these neighbors and put particle to this neighbor
^
particle.rb:40:29: C: Avoid parameter lists longer than 5 parameters.
def check_and_add_neighbor(x, y, size_x, lower_neighbors, drop_point, height_map)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
particle.rb:40:80: C: Line is too long. [83/79]
def check_and_add_neighbor(x, y, size_x, lower_neighbors, drop_point, height_map)
^^^^

1 file inspected, 11 offences detected

Similarly running :

reek particle.rb

Will produce :

particle.rb -- 20 warnings:
Particle has no descriptive comment (IrresponsibleModule)
Particle takes parameters [height_map, size_x, x, y] to 3 methods (DataClump)
Particle#agitate has 5 parameters (LongParameterList)
Particle#agitate has the parameter name 'x' (UncommunicativeParameterName)
Particle#agitate has the parameter name 'y' (UncommunicativeParameterName)
Particle#check_and_add_neighbor calls (x + (y * size_x)) 3 times (DuplicateMethodCall)
Particle#check_and_add_neighbor calls (y * size_x) 3 times (DuplicateMethodCall)
Particle#check_and_add_neighbor calls height_map[(x + (y * size_x))] twice (DuplicateMethodCall)
Particle#check_and_add_neighbor doesn't depend on instance state (UtilityFunction)
Particle#check_and_add_neighbor has 6 parameters (LongParameterList)
Particle#check_and_add_neighbor has the parameter name 'x' (UncommunicativeParameterName)
Particle#check_and_add_neighbor has the parameter name 'y' (UncommunicativeParameterName)
Particle#check_and_add_neighbor performs a nil-check. (NilCheck)
Particle#check_and_add_neighbor refers to height_map more than self (FeatureEnvy)
Particle#check_and_add_neighbor refers to x more than self (FeatureEnvy)
Particle#check_and_add_neighbor refers to y more than self (FeatureEnvy)
Particle#drop has 4 parameters (LongParameterList)
Particle#drop has the parameter name 'x' (UncommunicativeParameterName)
Particle#drop has the parameter name 'y' (UncommunicativeParameterName)
Particle#drop refers to height_map more than self (FeatureEnvy)

You can get a good feel for the differences between the two by scanning the types of errors produced.

In practice

While I am in favor of using such tools I also believe good judgement always trumps blindly following rules. It’s best to carefully select a subset of rules to adhere by rather than simply try to apply everyone of them. Also be ready to make exceptions even on those rules you have selected.

Trust developers to choose the best formatting for the code they are currently writing even if it goes against your general guidelines. They know more about the current context they are in than anyone.

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