Toxic Elephant

Don't bury it in your back yard!

Private Toolbox: An Anti-Pattern

Posted by matijs 10/04/2016 at 09h21

This is an anti-pattern that has bitten me several times.

Suppose you have an object hierarchy, with a superclass Animal, and several subclasses, Worm, Snake, Dog, Centipede. The superclass defines the abstract concept move, which is realized in the subclasses in different ways, i.e., by slithering or walking. Suppose that due to other considerations, it makes no sense to derive Worm and Snake from a SlitheringAnimal, nor Dog and Centipede from a WalkingAnimal. Yet, the implementation of Worm#move and Snake#move have a lot in common, as do Dog#move and Centipede#move.

One way to solve this is to provide methods walk and slither in the superclass that can be used by the subclasses that need them. Because it makes no sense for all animals be able to walk and slither, these methods would need to be accessible only to subclasses (e.g., private in Ruby).

Thus, the superclass provides a toolbox of methods that can only be used by its subclasses to mix and match as they see fit: a Private Toolbox.

This may seem an attractive course of action, but in my experience, this becomes a terrible mess in practice.

Let’s examine what is wrong with this in more detail. I see four concrete problems:

  • It is not always clear at the point of method definition what a method’s purpose is.
  • Each subclass carries with it the baggage of extra private methods that neither it nor its subclasses actually use.
  • The superclass’ interface is effectively extended to its non-public methods,
  • New subclasses may need to share methods that are not available in the superclass.

The Animal superclass shouldn’t be responsible for the ability to slither and to move. If we need more modes, we may not always be able to add them to the superclass.

We could extract the modes of movement into separate helper classes, but in Ruby, it is more natural to create a module. Thus, there would be modules Walker and Slitherer, each included by the relevant subclasses of Animal. These modules could either define move directly, or define walk and slither. Because the methods added in the latter case would actually makes sense for the including classes, there is less need to make them private: Once could make a instance of Dog walk, either by calling move, or by calling walk directly.

This solves all four of Private Toolbox’ problems:

  • The module names reveal the purpose of the defined methods.
  • Subclasses that do not need a particular module’s methods do not include it.
  • The implementor of Animal is free to change its private methods.
  • If a new mode of transportation is needed, no changes to Animal are needed. Instead, a new module can be created that provides the relevant functionality.

Tags , , no comments no trackbacks

Minimally Intrusive SimpleCov Loading

Posted by matijs 02/04/2016 at 16h55

I always like extra developer tooling to be minimally intrusive, to avoid forcing it on others working with the same code. There are several aspects to this: Presence of extra gems in the bundle, presence and visibility of extra files in the repository, and presence of extra code in the project.

For this reason, I’ve been reluctant to introduce tools like guard or some of the Rails preloaders that came before Spring. On the other hand, no-one would be bothered by my occasional running of RuboCop, Reek or pronto.

In this light, I’ve always found SimpleCov a little too intrusive: It needs to be part of the bundle, and the normal way to set things up makes it rather prominently visible in your test or spec helper. Nothing too terrible, but I’d like to just come to a project, run something like simplecov rake spec, and have my coverage data.

I haven’t reached that blissful state of casual SimpleCov use yet, but I’m quite pleased with what we achieved for Reek.

Here’s what we did:

  • Add simplecov to the Gemfile
  • Add a .simplecov file with configuration:
    SimpleCov.start do
      track_files 'lib/**/*.rb'
      # version.rb is loaded too early to test
      add_filter 'lib/reek/version.rb'
    end

    SimpleCov.at_exit do
      SimpleCov.result.format!
      SimpleCov.minimum_coverage 98.9
      SimpleCov.minimum_coverage_by_file 81.4
    end
  • Add -rsimplecov to the ruby_opts for our spec task:
    RSpec::Core::RakeTask.new('spec') do |t|
      t.pattern = 'spec/reek/**/*_spec.rb'
      t.ruby_opts = ['-rsimplecov -Ilib -w']
    end

This has several nice features:

First, there are no changes to spec_helper.rb. That file can get pretty cluttered, so the less has to be in there, the better.

Second, it only calculates coverage when running the full suite with rake spec. This means running just one spec file while developing won’t clobber your coverage data, and it makes running single specs a little faster since it doesn’t need to update the coverage reports.

Third, it enforces a minimum coverage per file and for the whole suite. The second point helps a lot in making this practical: Otherwise, running individual specs would almost always fail due to low coverage.

no comments no trackbacks