Good Module, Bad Module

Florian MotlikSeptember 9th, 2016Last Updated: September 8th, 2016

0 39 7 minutes read

You already know how to use modules in Ruby, but are you abusing them? In this post, we’ll take a look at different ways to program with modules and why they may or may not be a great idea.

Namespace

Modules give you an easy way to namespace the rest of your code. For example, when you generate a new gem:

$ bundle gem foo_bar_fighters

You get a default file with a module in it:

$ cat foo_bar_fighters/lib/foo_bar_fighters.rb
require "foo_bar_fighters/version"

module FooBarFighters
  # Your code goes here...
end

Now if you make a new class, you can put it in this FooBarFighters namespace:

module FooBarFighters
  class Song
  end
end

Now if you end up using another gem with a Song class, you can distinguish between the two by using FooBarFighters::Song.new.

Is this a good or a bad use of a module?

Since it’s auto generated by Bundler, you can bet that using a module as a namespace is a good idea. Modules used this way allow any gem or project to use any class name that they desire without risk of cross contamination.

That being said, if you have a very common name, like FooBarFighters::File, be careful; it’s not always clear if you’ll get your File constant or Ruby’s built-in constant. It’s best practice to not duplicate core class names even if they’re in a namespace. If you do have to, you can use the full constant FooBarFighters::File for your constant and ::File (with the :: in front) for Ruby’s built-in constant.

Method Sharing

You can put methods into a module and then “mix” (i.e., include) them into a class.

module Quack
  def say
    puts "quack"
  end
end

class WaterFowl
  include Quack
end

WaterFowl.new.say
# => "quack"

Is this good or bad module use?

This simple example is fine. However, there’s no real reason for the module. It would be simpler and there would be less code if we put that method right inside of the WaterFowl class.

class WaterFowl
  def say
    puts "quack"
  end
end

WaterFowl.new.say
# => "quack"

Since the module makes it more complicated, it’s bad, right? Well, let’s look at two separate examples from one of my projects: Wicked.

Sharing Behavior Through Modules

Wicked is a gem for building “step by step” wizard style controllers. It is a module, and you use use it by including it in your controller:

class AfterSignupController < ApplicationController
  include Wicked::Wizard

  # ...
end

Note the namespace, yay!

It makes sense for Wicked to contain methods that are mixed into a controller since it can be mixed into multiple controllers.

More than just methods though, the Wicked::Wizard module contains a set of behaviors that we want to share. When a user clicks on a specific link, we want them to go to the next page in the wizard flow. This is a good use of sharing methods via a module. In this case, the interface is provided by the module.

A benefit of this approach is that multiple interfaces can be layered on the same class. Originally, when I made Wicked, the interface was provided by a class that you had to inherit from.

class AfterSignupController < Wicked::WizardController
  # ...
end

This was not so great since it limits the ability to inherit from other custom classes (because Ruby only supports single inheritance). By putting our behavior in a module, we allow for a kind of multiple inheritance:

class AfterSignupController
  include Wicked::Wizard
  include SetAdmin
  include MailAfterSuccessfulCreate
  include FooBarFighters::MyHero
  # ...
end

Module Method Extraction

Another use of splitting out methods can be seen in the Sprockets gem, which I currently maintain. Sprockets split up lots of behavior into modules; here’s a taste of a few of the modules:

Sprockets::HTTPUtils
Sprockets::Mime
Sprockets::Server
Sprockets::Resolve
Sprockets::Loader
Sprockets::Bower
Sprockets::PathUtils
Sprockets::PathDependencyUtils
Sprockets::PathDigestUtils
Sprockets::DigestUtils
Sprockets::SourceMapUtils
Sprockets::UriUtils

Each of these modules eventually makes its way to the class Sprockets::Environment, which is wrapped by Sprockets::CachedEnvironment. When you instantiate the object, it has 105 different methods. People call classes like this “God objects” since they seem to be all powerful. The Sprockets::Environment class definition in the source code is only 27 lines long without comments:

module Sprockets
  class Environment < Base
    def initialize(root = ".")
      initialize_configuration(Sprockets)
      self.root = root
      self.cache = Cache::MemoryStore.new
      yield self if block_given?
    end

    def cached
      CachedEnvironment.new(self)
    end
    alias_method :index, :cached

    def find_asset(*args)
      cached.find_asset(*args)
    end

    def find_all_linked_assets(*args, █)
      cached.find_all_linked_assets(*args, █)
    end

    def load(*args)
      cached.load(*args)
    end
  end
end

This is bad. We only have five methods in this file. Where did the other 100 methods come from? Splitting out methods into modules just to mix them back into one God object doesn’t reduce complexity; it makes it harder to reason about.

I admit that at one point in time, “concerns” — the practice of splitting out behavior for one class into many modules — was in vogue, and I too indulged in it. I even have a concerns folder in Wicked. While it’s fine to expose an interface of Wicked::Wizard via a module, the project (and docs) do not expect you to directly include these modules:

Wicked::Controller::Concerns::Action
Wicked::Controller::Concerns::Path
Wicked::Controller::Concerns::RenderRedirect
Wicked::Controller::Concerns::Steps

These are then all mixed into Wicked::Wizard. Splitting things up doesn’t make things easier — it only means I can pretend that my files are small and that my main module doesn’t introduce that many methods. Even though I wrote the code, I’m constantly confused about which file holds what method. If anything, splitting out files in this way makes my job as a maintainer harder.

I previously wrote about reusing code with concerns and legacy concerns in Rails. If you haven’t read them, don’t. Extracting methods into a module to only turn around and include them is worse than pointless. It’s actively damaging to the readability of your codebase.

Global Methods

When you put a method on a module directly (i.e., using def self or extend self), then it becomes a global method. You can see this in Sprockets::PathUtils.entries method:

module Sprockets
  module PathUtils
  # ...

  def self.entries(path)
    if File.directory?(path)
      entries = Dir.entries(path, encoding: Encoding.default_internal)
      entries.reject! { |entry|
        entry.start_with?(".".freeze) ||
          (entry.start_with?("#".freeze) && entry.end_with?("#".freeze)) ||
          entry.end_with?("~".freeze)
      }
      entries.sort!
      entries
    else
      []
    end
  end
end

Code changed slightly for clarity of example.

This method finds all the files in a given directory and does not include any hidden files and returns a sorted array.

You can use this method globally:

Sprockets::PathUtils.entries("/weezer/blue-album")
# => ["Buddy Holly", "Holiday", "In the Garage", "My Name Is Jonas No One Else", "Only in Dreams", "Say It Ain't So", "Surf Wax America", "The World Has Turned and Left Me Here", "Undone – The Sweater Song"]

The pros here are that the method can be used globally. This simple functionality can be tapped into by any piece of code without having to include the module. That means other code only has to know about the methods it needs instead of getting all the methods in the PathUtils module.

The downside is that the method can be used globally; its scope isn’t limited. If you need to change the behavior — let’s say you need to return the array in reverse sorted order — you might break other code that you didn’t even know was relying on this method.

Is sharing methods globally by using methods directly on classes good or bad?

Ideally you wouldn’t need a global method, but sometimes it makes sense as an API. One example is FileUtils. I use this all the time to make directories:

require 'fileutils'

FileUtils.mkdir_p("/ruby/is/awesome/lets/make/directories")

This is a global method, and FileUtils is a module:

puts FileUtils.class
# => Module

I’ll say that using modules for global methods is better than for building God objects. How so? If you look at the code:

def stat_digest(path, stat)
  if stat.directory?
    # If its a directive, digest the list of filenames
    digest_class.digest(
      self.entries(path) # <===================
    .join(','.freeze))

I would prefer if it was more explicit:

def stat_digest(path, stat)
  if stat.directory?
    # If its a directive, digest the list of filenames
    digest_class.digest(
      PathUtils.entries(path) # <===================
    .join(','.freeze))

In the second example, we know exactly where to look for our implementation (in the PathUtils module) since it’s right in front of us. We also know that the method call isn’t mutating anything that is not passed in. For example, PathUtils could be mutating the path argument but nothing else. It doesn’t have access to any of the rest of the current scope.

We still haven’t said if sharing global methods via modules is good or bad. If there is no way you can work around needing a global method, what are other options for exposing a method globally?

You could define it in a top level context:

$ irb
> def nirvana
    puts "come as you are"
  end

> class Foo
    def say
      nirvana
    end
  end

>  Foo.new.say
# => "come as you are"

You could also “monkey patch” it into Object or Kernel:

class Object
  def nirvana
    puts "come as you are"
  end
end

class Foo
  def say
    nirvana
  end
end
Foo.new.say
# => "come as you are"

We can agree (hopefully) that both of these approaches are more surprising and more invasive to the end user than stashing our global methods in a module. We’ll say that using modules for global methods is “good.”

Bad Modules Make for Good Objects

Many of the “bad” module practices (including my own) came from a somewhat limited understanding of object-oriented design.

You may hear things like “never have a file longer than [some number] lines of code,” which leads you to think that taking the existing methods, splitting them out into modules and including them is the quick fix. While it will give you shorter files, it also makes things worse. It kills readability and grepability.

A well-written class can be a work of art, while most modules tend to be proverbial junk drawers. This isn’t a post on OO design though, so I’ll punt on that. If you want to know more, look up “refactoring confreaks Ruby,” and you’ll find a ton of material. One notable book on the subject is Practical Object Oriented Design in Ruby by Sandi Metz.

Modules are a valuable tool in your Ruby toolbox. But next time you reach for a module to solve a problem, ask yourself if it’s a “good” fit for the job.

Reference:

Good Module, Bad Module from our WCG partner Florian Motlik at the Codeship Blog blog.