The letter A styled as Alchemists logo. lchemists
Published September 1, 2020 Updated October 23, 2023
Cover
Ruby Antipatterns

An antipattern is just like a pattern, except that instead of a solution it gives something that looks superficially like a solution but isn’t one. — Andrew Koenig

The Ruby language is rich in functionality, expressiveness, and succinctness, a blend of object-oriented and functional styles. Because of this unique construction, Ruby can have sharp edges that makes your codebase hard to read, debug, and maintain. The following dives into a few such aspects of the Ruby language that should be avoided in professional work. Don’t get me wrong! It’s fun to play with the following problematic examples as local experiments and/or explorations for personal or even debugging purposes. However, when it comes time to committing code that will be used for production purposes, these practices should be avoided.

Class Variables

Class variables behave a lot like global variables — discussed later in this article — except they are scoped within their own class hierarchy rather than globally. For a closer look, consider the following:

class Parent
  @@example = "defined by parent"

  def self.print = puts(@@example)
end

class Child < Parent
end

Parent.print # "defined by parent"
Child.print  # "defined by parent"

Notice both the Parent and Child classes yield the same output, which seems reasonable until the following modification is introduced:

class Child < Parent
  @@example = "overwritten by child"
end

Parent.print # "overwritten by child"
Child.print  # "overwritten by child"

Not only did the Child define it’s own value but it mutated the Parent as well. If you haven’t seen this before, it can be an unpleasant surprise. A quick and dirty solution would be to instead use a class instance variable:

class Parent
  @example = "defined by parent"

  def self.print = puts(@example)
end

class Child < Parent
  @example = "defined by child"
end

Parent.print # "defined by parent"
Child.print  # "defined by child"

While the above is an improvement, a better approach would be remove state from classes altogether and encapsulate behavior in a barewords instance:

class Parent
  def initialize message = "parent default"
    @message = message
  end

  def print = puts(message)

  private

  attr_reader :message
end

class Child < Parent
  def initialize message = "child default"
    super
  end
end

Parent.new.print # "parent default"
Child.new.print  # "child default"

As an added bonus, you now have a way to construct multiple instances of each class with different default messages.

It is worth noting that with Ruby 3.0.0 you’ll get a RuntimeError if attempting to re-assign a variable owned by another class in the hierarchy:

class Parent
  def self.mutate! = @@example = []
end

class Child < Parent
  @@example = {}

  def self.example = @@example
end

Parent.mutate!
Child.example  # RuntimeError (class variable @@example of Child is overtaken by Parent)

The same is true, with Ruby 3.0.0, if a module attempts to change the class variable:

class Demo
  @@example = []

  def self.example = @@example
end

module DemoModule
  @@example = []
end

Demo.include DemoModule
Demo.example  # RuntimeError (class variable @@example of Demo is overtaken by DemoModule)

Even with improved support in Ruby 3.0.0, use of class variables should be avoided. As an aside, there was a glimmer of hope that class variables would be removed from the language entirely but Matz stepped in with the final word:

I admit class variables semantics are a bit complex and sometimes misunderstood. But removing them should cause serious compatibility issues.

Class variables are somewhat similar to global variables. We don’t recommend using/abusing them, but not going to remove them from the language.

Flat Modules

A flat module structure looks like this:

module One
end

module One::Two
end

module One::Two::Three
  puts Module.nesting # [One::Two::Three]
end

Notice when we print the module structure at point of call we get a single element array: One::Two::Three. To contrast the above, here’s the result when defining nested modules:

module One
  module Two
    module Three
      puts Module.nesting # [One::Two::Three, One::Two, One]
    end
  end
end

Now we have a three element array where each module in the hierarchy is a unique element in the array. Our object hierarchy is preserved so constant/method lookup is relative to the hierarchy. To illustrate further, consider the following:

module One
  EXAMPLE = "test"
end

module One::Two
end

module One::Two::Three
  puts EXAMPLE
end

# uninitialized constant One::Two::Three::EXAMPLE (NameError)
# Did you mean?  One::EXAMPLE

As the error above shows, we could use One::Example to solve the problem. To not repeat ourselves, use a nested module structure instead:

module One
  EXAMPLE = "test"

  module Two
    module Three
      puts EXAMPLE
    end
  end
end

Notice how the above is concise, removes duplication of the earlier example, and provides a unique constant for each level of the hierarchy for relative reference and lookup. Stick to nested modules so you can avoid needless implementation construction and unexpected errors.

Global Variables

Ruby has a collection of global variables which are borrowed from Perl and shell scripting, in general. These can be viewed via the Kernel#global_variables method. A full description of all variables and their corresponding aliases can be found within the English library source code (i.e. require "English"). Here’s a rough breakdown of each:

$/       # Input record separator. Alias: $INPUT_RECORD_SEPARATOR. Default: newline. Deprecated in Ruby 2.7.0.
$.       # Current input line number of the last file read. Alias: $INPUT_LINE_NUMBER.
$\       # Output record separator. Alias: $OUTPUT_RECORD_SEPARATOR. Default: nil. Deprecated in Ruby 2.7.0.
$;       # String#split default field separator. Alias: $FIELD_SEPARATOR. Deprecated in Ruby 2.7.0.
$,       # Output field separator. Alias: $OUTPUT_FIELD_SEPARATOR. Deprecated in Ruby 2.7.0.
_$       # Input variable for each object within an IO loop.
$!       # Last exception thrown. Alias: $ERROR_INFO.
$@       # Backtrace array of last exception thrown. Alias: $ERROR_POSITION.
$&       # String match of last successful pattern match for current scope. Alias: $MATCH.
$`       # String to the left of last successful match. Alias: $PREMATCH.
$'       # String to the right of last successful match. Alias: $POSTMATCH.
$+       # Last bracket matched by the last successful match. Alias: $LAST_PAREN_MATCH.
$<n>     # nth group of last successful regexp match.
$~       # Last match info for current scope. Alias: $LAST_MATCH_INFO.
$<       # Object access to the concatenation of all file contents given as command-line arguments. Alias: $DEFAULT_INPUT.
$>       # Output destination of Kernel.print and Kernel.printf. Alias: $DEFAULT_OUTPUT. Default: $stdout.
$_       # Last input line of string by gets or readline. Alias: $LAST_READ_LINE.
$0       # Name of the script being executed. Alias: $PROGRAM_NAME.
$-       # Command line arguments given for script. Alias: $ARGV.
$$       # Ruby process number of current script. Alias: $PROCESS_ID or $PID.
$?       # Status of the last executed child process. Alias: $CHILD_STATUS.
$:       # Load path for scripts and binary modules via load or require. Alias: $LOAD_PATH.
$"       # Array of module names as loaded by require. Alias: $LOADED_FEATURES.
$-d      # Status of the -d switch. Alias: $DEBUG.
$-K      # Source code character encoding being used. Alias: $KCODE.
$-v      # Verbose flag (as set by the -v switch). Alias: $VERBOSE.
$-a      # True if option -a ("autosplit" mode) is set.
$-i      # In-place-edit mode. Holds the extension if true, otherwise nil.
$-l      # True if option -l is set ("line-ending processing" is on).
$-p      # True if option -p is set ("loop" mode is on).
$-w      # True if option -w is set (Ruby warnings).
$stdin   # Current standard input.
$stdout  # Current standard output.
$stderr  # Current standard error output.

You can also define and use your own global variables:

$example = "hello"
puts $example # "hello"

Global variables are good to be aware of but suffer from several issues:

  • Encourages poor coding practices.

  • Are difficult to read and intuit their meaning.

  • Suffer from a higher chance of variable conflict due to using the same global namespace.

  • Breaks encapsulation because the variable can then be used anywhere within the program and even mutated which leads to unexpected bugs, difficultly in debugging, and resolving issues.

Requiring the English library will improve readability but doesn’t prevent the values from being mutated unless you remember to freeze them. A better approach is to reduce scope to the objects that need these values through dependency injection via default constants or even default configurations provided by gems like XDG or Runcom.

Insecure Constantization

In some situations, it can be tempting to automatically load a constant when a constant is not found. Example:

module Example
  # Security exploit since any object within this namespace could be loaded.
  def self.const_missing(name) = puts("Attempted to load: #{self}::#{name}.")
end

Example::Danger  # Attempted to load: Example::Danger.

The problem with the above is that this can lead to malicious behavior where a hacker could exploit the above by attempting to load unexpected code. Here are further examples:

module Example
  # Memory exploit since constants are never garbage collected.
  def self.const_missing(name) = const_set(name, "default")
end

Example::ONE  # "default"
Example::TWO  # "default"
module Example
# Larger memory exploit since these objects will not be garbage collected either.
  def self.const_missing(name) = const_set(name, Class.new(StandardError))
end

Example::One  # Example::One
Example::Two  # Example::Two

Dynamically loading constants or other objects can be useful in limited situations. To prevent malicious attacks, ensure safety checks are put in place. For example, all of the above could be avoided with the following check:

module Example
  ALLOWED_CONSTANTS = %i[ONE TWO]

  def self.const_missing name
    fail StandardError, "Invalid constant: #{name}." unless ALLOWED_CONSTANTS.include?(name)
    # Implementation details go here.
  end
end

Example::ONE    # nil
Example::BOGUS  # `const_missing': Invalid constant: BOGUS. (StandardError)

The dynamic nature of Ruby provides great power but with this this power comes great responsibility. When reaching for functionality like this, keep usage limited along with restricted access to narrow any potential for exploit.

Insecure Open

The Kernel module is powerful and convenient module which provides syntactic sugar for common situations that requires less typing and/or setup if Kernel did not exist. That said, it comes with sharp edges. One of them being Kernel.open which provides a quick way to open a file or URL. The only problem this is method is cause for a lot of security issues which are best explained and documented in this Ruby Issue.

You are much better off using IO.popen or URI.open in these situations since they are more secure.

Insecure Templates

When using ERB (or any templating language supported by Tilt), ensure you avoid using <%== %> or <%= raw %> in your templates or, if your framework provides it, #html_safe. All of these techniques avoid proper sanitization of malicious content, usually from user input, which is a huge security risk to your product/service. Instead, lean on your framework to sanitize input so you can avoid Cross-Site Scripting attacks altogether. Usually your framework will provide this for you via a #sanitize method. Whatever it might be called, please use it.

Jumbled Message Chains

When chaining multiple messages together, default to using a single line as long as the line doesn’t violate your style guide’s column count. In situations where you need to use multiple lines for message changes — and this is important — ensure all messages are vertically aligned by dot notation so it’s easier to read through each step of the message chain (as shown in the examples below). For instance, take the following which reads from a file and splits on new lines:

lines = path.read.split "\n"

The above is short and readable but if the implementation became more complex, you’d risk violating your column count and incur a horizontal scrolling cost to read through the implementation. By breaking up your messages per line and vertically aligned by dot notation, you improve readability. Example:

lines = path.read
            .split("\n")
            .map(&:strip)
            .reject(&:empty?)
            .uniq
            .sort

You’ll also want to break your message chains into single lines when using anonymous functions. Example:

# Avoid
lines = path.read.split("\n").tap { |lines| logger.debug "Initial lines: #{lines}." }.map(&:strip)

# Use
lines = path.read
            .split("\n")
            .tap { |lines| logger.debug "Initial lines: #{lines}." }
            .map(&:strip)

While the one-liner is syntactically correct, it decreases readability. Even worse, imagine a one-liner with multiple anonymous functions all chained together on a single line. The readability and maintainability would get out of hand rather quickly. One common example of multiple anonymous functions chained together is when using RSpec. Take the following example:

it "adds membership" do
  expect {
    organization.memberships.create user: user
  }.to change {
    Membership.count
  }.by(1)
end

The above abuses dot notation to chain together multiple anonymous messages. A better solution is to use local variables to reduce complexity. Example:

it "adds membership" do
  expectation = proc { organization.memberships.create user: user }
  count = proc { Membership.count }

  expect(&expectation).to change(&count).by(1)
end

While the above incurs a local variable cost, the trade-off is that the entire expectation is readable via a single line. 🎉

There is an obscure style worth mentioning which is Jim Weirich’s semantic rule for {} versus do...end. The semantic rule is less common and much harder to enforce. Plus, as you’ve seen above, there is value in being able to use {} for message chains that might return a value or cause a side effect in terms of readability and succinctness.

Finally — and related to the above — when needing to chain a multi-line anonymous function, ensure the call is at the end of a message chain. Example:

# Avoid
lines = path.read
            .split("\n")
            .tap do |lines|
              logger.debug "Initial lines: #{lines}."
              instrumenter.instrument "file.read", count: lines.size
            end
            .map(&:strip)

# Use
lines = path.read
            .split("\n")
            .map(&:strip)
            .tap do |lines|
              logger.debug "Initial lines: #{lines}."
              instrumenter.instrument "file.read", count: lines.size
            end

While the above is a contrived example, you see how readability improves as the eye traces — top to bottom — through the dot notation with the multi-line do…​end signaling the end of the chain. Should you need multi-line functionality in the middle of your message chain, then you can give this functionality a name by using a method. Example:

def record lines
  logger.debug "Initial lines: #{lines}."
  instrumenter.instrument "file.read", count: lines.size
end

lines = path.read
            .split("\n")
            .tap { |lines| record lines }
            .map(&:strip)

Lazy Operator

The lazy operator — or more formally known as the Safe Navigation Operator — was introduced in Ruby 2.3.0. This operator is particularly egregious because it makes code hard to read and leads to defensive coding when we could be writing Confident Code instead. Here’s an example:

Example:

author = nil
author&.address&.zip  # nil

When you take the above and decompress it, you end up with the following nested logic:

if auther
  if auther.address
    auther.address.zip
  else
    nil
  end
else
  nil
end

That’s a lot of defensive coding because with author&.address&.zip we have a hard time discerning what this code is trying to say:

  • Is it OK if author starts out as nil because once we start with nil, there is nothing stopping nil propagating through the entire message chain?

  • Is it because we are uncertain whether author will be present or maybe only address could potentially be nil and author is an unfortunate defensive guard?

The above violates/encourages all of the following:

  • Connascence of Name/Type - We now have a tight coupling to the names of the objects via the messages being sent. We also need to know the object hierarchy and types of objects we are messaging (i.e. address and zip). If this code were to be refactored so the names and/or types changed, the above code snippet would still work. This false positive can be hard to detect if not thoroughly tested or, worse, discovered in production only.

  • Null Object - Instead of messaging the author for address and then zip, address could answer back a null object that acts like an address but has safe default values. Same goes for zip. At least, in this case, you’d be dealing with the same types of objects for validation purposes.

  • Broken Window - Once a nil has been introduced, the use of the nil tends to leak into more code that needs to deal with a nil thus perpetuating more broken windows within the code base. This is also known as the Billion Dollar Mistake.

  • Law of Demeter - This harkens back to the connascence issue where we have to dig into the author and then address to pluck out the value of zip. We could teach author to answer back the zip. Example: author.zip. Now the underlying semantics can be refactored as necessary with the calling object being none the wiser. Even better, the Adapter Pattern could be used to consume an address and provide a #zip message with the appropriate validation and error handling.

  • NoMethodError: undefined method for nil:NilClass - Finding a solution becomes significantly harder because the root cause of NoMethodError is typically obscured several layers down the stack where the lazy operator was first introduced.

💡 You can ensure the Lazy Operator is never used by adding the Caliber gem to your project as part of your code quality checks.

Misused Casting

You can cast objects to different types through explicit (standard) and implicit (rare) casting. Here’s a quick breakdown of primitive types with their explicit and implicit equivalents:

Type Explicit Implicit

Integer

to_i

to_int

String

to_s

to_str

Array

to_a

to_ary

Hash

to_h

to_hash

Where this goes terribly wrong is when implicit casting is misused as the object’s primary API. Example:

class Version
  def initialize major, minor, patch
    @major = major
    @minor = minor
    @patch = patch
  end

  def to_str = "#{major}.#{minor}.#{patch}"

  private

  attr_reader :major, :minor, :patch
end

version = Version.new 1, 2, 3
puts "Version: #{version.to_str}"  # "Version: 1.2.3"

Notice the implicit #to_str method is explicitly called. Ruby will happily evaluate your implementation but explicitly messaging an implicit method, as shown above, should be avoided. Here’s a more appropriate implementation:

class Version
  def initialize major, minor, patch
    @major = major
    @minor = minor
    @patch = patch
  end

  def to_s = "#{major}.#{minor}.#{patch}"

  alias_method :to_str, :to_s

  private

  attr_reader :major, :minor, :patch
end

version = Version.new 1, 2, 3
puts "Version: " + version # "Version: 1.2.3"

Notice #to_s is the explicit implementation provided for downstream callers while the implicit #to_str method is an alias of #to_s so Ruby can perform the implicit casting of version to a string when printed on the last line. Without the implicit alias, you’d end up with a TypeError.

The main point here is to always default to implementing the explicit version of the type you desire while only aliasing implicit support if you want to allow Ruby to conveniently cast your object into another type without having to force explicit use.

Should you want a fully fledged implementation of the Version implementation show above with additional examples, I encourage you to check out the Versionaire gem.

Misused Class Methods

Class methods — also known as singleton methods — often get abused when used to answer literals and static values in general. Example:

class User
  def self.role = "visitor"
end

While tempting to use a class method for this information, a constant is meant for this exact purpose which means the above can be rewritten as follows:

class User
  ROLE = "visitor"

  attr_reader :role

  def initialize role: ROLE
    @role = role
  end
end

Notice, with the above, that the ROLE constant is now used to define the default role which then can be easily injected and asked for via the #role instance method. This provides several advantages:

  • At any time, you can ask for a user’s default role via User::ROLE.

  • Avoids class methods being used to answer raw data because this is what constants are best used for.

  • Allows you to inject the value of your instance via the initializer so you can more easily create multiple user instances with different roles.

Misused Endless Methods

Introduced in Ruby 3.0.0, these should not be used for multi-line message chains:

def read(path) = path.read
                     .split("\n")
                     .map(&:strip)
                     .reject(&:empty?)
                     .uniq
                     .sort

Instead, it is better to use a normal method if multiple lines are necessary:

def read path
  path.read
      .split("\n")
      .map(&:strip)
      .reject(&:empty?)
      .uniq
      .sort
end

Even better, if an endless method can fit on a single line then use the single line:

def read(path) = path.read.split("\n").map(&:strip).reject(&:empty?).uniq.sort

This improves readability, ensures endless methods remain a single line, and doesn’t cause exceptions when copying and pasting in IRB. To see what I mean, try copying and pasting the multi-line endless method above in IRB while the second example, using a normal block, will not error.

Misused Method Parameters

Related to Redundant Naming is the misuse of method parameters. The pattern you want to follow when defining method parameters is (listed in order of precedence):

  1. Positional (optional)

  2. Positional (required)

  3. Positional (single splat)

  4. Keyword (optional)

  5. Keyword (required)

  6. Keyword (double splat)

  7. Block

💡 Read the Method Parameters And Arguments article to learn more.

Consider the following API client:

class Client
  def initialize url:, http:
    @url = url
    @http = http
  end
end

Notice both url and http are required keyword parameters which means we can’t easily construct a new client via Client.new which makes obtaining an instance of this object much harder. Each time we need an instance of this client, we’d have to type the following:

client = Client.new url: "https://api.example.com", http: HTTP

That’s overly verbose for little gain. Here’s an alternative approach that provides an improved developer experience:

require "http"

class Client
  URL = "https://api.example.com"

  def initialize url = URL, http: HTTP
    @url = url
    @http = http
  end
end

Notice url is a positional parameter with a safe default while http is keyword parameter with a safe default. The distinction is subtle but powerful. Consider the following usage:

client = Client.new                                          # Fastest usable instance.
client = Client.new "https://api.other.io"                   # Instance with custom API URL.
client = Client.new http: Net::HTTP                          # Instance with custom HTTP handler.
client = Client.new "https://api.other.io", http: Net::HTTP  # Fully customized.

Due to the nature of an API client always needing to interface with an URL, you can avoid stating the obvious by making url a positional instead of keyword parameter. On the flip side, http needs to be a keyword parameter because it’s not entirely intuitive the second parameter is meant for injecting a different HTTP handler. In both cases, the positional and keyword parameter are optional which is a nice win in terms of usability without sacrificing readability.

Monkey Patches

Ruby has always allowed objects to be be monkey patched. Example:

class String
  def split(pattern = nil) = fail StandardError, "Sorry, I have no idea how to split myself. 😈"
end

In the above, we monkey patch the String class by opening it to add new behavior. Unfortunately, this new behavior now applies to all strings within the application that need to split themselves. Here’s the result of this change (truncated for brevity):

"example".split  # => Sorry, I have no idea how to split myself. 😈 (StandardError)

With the above example, we at least have a stack dump, despite not being shown, to trace back to where the monkey patch was introduced. What if the change was more subtle?

class String
  def size = 13
end

Upon using the above monkey patch, we see the following output:

"example".size # => 13

Unfortunately, String#size always answers 13 now. Imagine if this code wasn’t in our application but was loaded from a gem dependency or — worse — indirectly via a gem’s own dependency. Beyond the struggle of identifying the root cause, the above also breaks expected behavior understood by all Ruby engineers, leading to time lost debugging confusing behavior not seen before.

All of the above leads to the following problems that will multiply and/or compound upon themselves in problematic ways:

  • They globally alter the behavior of your entire application which makes them harder to maintain, debug, or remove later. Managing global behavior always leads to brittle and hard to maintain code.

  • They lead to surprising behavior — as illustrated above — where members of your team can spend hours, even days, trying to debug and understand why undocumented behavior or behavior common to Ruby core doesn’t behave as expected.

  • They make upgrading your dependencies and applications harder because of the difficult effort required to upgrade combined with having to fix any/all monkey patches. When you can’t easily upgrade or maintain your app on a weekly basis (minimum), you quickly fall into a downward spiral of not being able to utilize new features and apply the latest security fixes for your customers.

  • They prevent you from being a good Open Source collaborator in that any monkey patch you apply is local your stack but others in the community could benefit from the patch as well. Submitting an upstream patch, instead of monkey patching, allows the entire community to benefit from your work and releases you from maintaining additional lines of code.

  • To bolster the above arguments further, consider Piotr Solnica’s article on why Rails is not written in Ruby where he explains the implications of taking monkey patches too far.

Here’s how you can avoid monkey patching altogether:

  • Start with fixing upstream dependencies by issuing a open source patch so the greater community benefits.

  • Fallback to using Refinements, which were fully implemented in Ruby 2.1.0. They allow you to lexically scope changes to only the code that needs the modification. Even better, refinements allow you see exactly where new behavior was introduced and are much easier to maintain.

Multi-Object Files

Ruby has no limitation with defining multiple objects per file, as shown in the following:

# demo.rb

module One
  module Two
  end
end

class Example
end

There are several disadvantages to this lack of limitation:

  • The One::Two module and Example class are defined in a demo.rb file which is not intuitive when searching for either via a fuzzy file search.

  • The implementation suffers from poor organization as none of the above are remotely related from a naming standpoint. Even if the objects were loosely related, it is better to isolate to a separate file as mentioned in the previous bullet point.

  • The example above, while momentarily small, can get unwieldy to read/maintain if the size of each implementation grows in complexity.

Rather than multiple objects per file, a better solution would be to define each object in a separate file, like so:

# one/two.rb
module One
  module Two
  end
end
# example.rb
class Example
end

Organizing one object per file helps encourage a consistent structure and reduces the strain of finding the associated file with the implementation you are looking for.

Multiple Inheritance

Multiple inheritance — or mixins — are a bad idea that are often abused. In the worst cases, they are used to extract common functionality from multiple objects as a way to share methods or DRY common functionality while not solving the problem of proper encapsulation at all. What you want to reach for is SOLID design and/or a more functional approach via the Command Pattern. One illustrative example of how bad this can get can be found in this article on A Kautionary Tale About Mixins. Again, stick to using the Single Responsibility Principle and Dependency Injection from SOLID and you’ll be much better off.

Nested Classes

You can nest classes within each other and still be able to reference them anywhere in your application without fear of being scoped only to the parent namespace. This behavior is very different from languages, like Java for example, where an inner class is scoped to the outer class only. Here’s an example:

class Outer
  class Inner
    def self.speak
      puts "HI"
    end
  end
end

Outer::Inner.speak # "HI"

A module is similar to using a class:

module Outer
  class Inner
    def self.speak
      puts "HI"
    end
  end
end

Outer::Inner.speak # "HI"

In both cases above, the difference is between Outer being defined as either a class or a module. Both are acceptable and both are constants that give structure to your code.

The problem with nested classes arises when you truly want to use modules to organize your code and stumble upon some random nested class in your code base that has the same name as your module. This can be frustrating from an organization and refactoring standpoint since Ruby code is typically organized in modules. Here’s an example, along with an associated error, in which we attempt use a module without realizing a class of the same name existed:

class Outer
  class Inner
  end
end

module Outer
  class Other
  end
end

# 6:in `<main>': Outer is not a module (TypeError)
# 1: previous definition of Outer was here

For this reason alone, it’s better to avoid nesting your classes and use module for organizing your code, reserving class for your actual implementation.

Nonclasses

A nonclass is a class with methods which have no state, behavior, or any relation to the class in which the methods were defined. Consider the following:

class Pinger
  def call(url) = HTTP.get(url).status
end

There are a several issues with the above implementation:

  • There is no constructor for instantiating an instance of a class.

  • The #call method has no ties to the class since there is no state or behavior that makes the #call method unique to the class. This method could be randomly added to any class.

  • The implementation is hard to test because there is no way to spy on the HTTP object.

To correct the above, ensure the implementation is less brittle by making it more flexible through dependency injection:

class Pinger
  def initialize client: HTTP
    @client = client
  end

  def call(url) = client.get(url).status

  private

  attr_reader :client
end

With the above, the #call method has more relevance due to depending on the injected client. Even better, you can swap out the HTTP client implementation with any object that responds to #get and answers a response with a #status giving you more flexibility in how you might ping a remote server to check its status.

Numbered Parameters

These were introduced in Ruby 2.7.0. Here’s a simple example:

["Zoe Alleyne Washburne", "Kaywinnet Lee Frye"].each { puts _1 }

# Yields:
# Zoe Alleyne Washburne
# Kaywinnet Lee Frye

To improve readability, you could write it this way:

["Zoe Alleyne Washburne", "Kaywinnet Lee Frye"].each { |character| puts character }

While this solution requires more typing, good variable names help convey meaning and serve as a form of documentation. In this case, we’re not only printing names of people but we can see these are fictional characters. The value of this cannot be understated, especially if you were to iterate all enumerables using numbered parameters throughout your entire codebase. The use of _1, _2, _3, etc. would get monotonous quickly and make it hard to maintain context of each code block.

Nested blocks aren’t supported either because the following fails:

1.times do
  _1.times { puts _1 }
end

# Yields:
# SyntaxError...numbered parameter is already used in...outer block

Similarly, you can’t use _1 as a variable name — not that you’d want to anyway. Example:

_1 = "test"
# Yields: warning: `_1' is reserved for numbered parameter; consider another name

Focus on readability. Your fellow colleagues and future self will thank you.

Obvious Comments

While the original intent of adding comments to your code is well meaning, you want to avoid adding them since they tend to become stale, misleading, and cause misdirection when reading or debugging code. Instead, focus on refactoring your code so the code itself is self-describing or educate your team if they don’t understand certain concepts, patterns, and/or paradigms. To explain further, consider the following code comments:

# Argument forwarding is used to delegate to the instance while providing class level convenience.
def self.call(...) = new(...).call

# The delegate is fetched from the injected delegates based on current method name and then called.
def build = delegates.fetch(__method__).call

# Tap is being used to log debugging information as a side effect within this pipeline.
my_string.split(",")
         .tap { |array| logger.debug "The array is: #{array}."}
         .map(&:upcase!)

All of the above examples read as if Captian Obvious wrote them. You could also be thinking: "Hey, wait, I don’t know what #tap, argument forwarding, or _method_ is so I need these comments!". Again, no you don’t. Ruby is a powerful language and just because you’ve not used certain aspects of the language doesn’t mean you should add comments to explain the code. All you probably need is a pointer to other examples within the code base or documentation to get up to speed. Once empowered with this new knowledge, you can then teach these techniques to other less experienced colleagues. This allows the team to keep leveling up while avoiding unnecessary and redundant documentation.

There are times where code comments are warranted, though. For example:

DELEGATES = [Ingress, Combinator, Reducer, Egress].freeze  # Order matters.

gem "debug", "1.4.0"  # TODO: Patch pin is required until Ruby team fixes bug with 1.5.0.

# FIX: This was written long ago, is complicated, and needs to be refactored or removed entirely.
def some_complicated_method
  # Implementation details.
end

All of the above comments are valuable because they provide information that isn’t obvious and provide pointers on how to use or enhance the code further. In the case of the TODO and FIX comments, while not something you want to leave around for long periods of time, they do provide next actions for further code cleanup.

In summary, be judicious with your use of code comments, focus on self-describing code, and spread your knowledge across the team so everyone keeps leveling up.

Privacy Invasion

Private and protected methods communicate the following in object design:

  • Private

    • Functionality supports the current object only.

    • Method is off limits and potentially volatile.

    • Implementation and method signature might change.

  • Protected

    • Method is not available for public use.

    • Method is only available for use by the parent class and all subclasses.

While the following is a contrived example — and would apply to protected methods too — notice how private clearly delineates between the objects’s public versus private API:

class WordBuilder
  DELIMITER = "-"

  def initialize delimiter: DELIMITER
    @delimiter = delimiter
  end

  def call(text) = split(text).map(&:capitalize)

  private

  attr_reader :delimiter

  def split(text) = text.split(delimiter)
end

While you can use BasicObject#__send__ to message the #delimiter or #split private methods, you should avoid doing this for all of the reasons mentioned above. This includes testing a private or protected method. To emphasize further, avoid the following:

  • Never break encapsulation by reaching into an object to message a private or protected method via BasicObject#__send__. BasicObject#__send__ should only be used by the object that implements the private method since the object owns it.

  • Never test an object’s private or protected methods in your specs. These methods can be tested indirectly via the object’s public API instead.

Redundant Naming

Naming objects, methods, parameters, etc. can be a difficult task when implementing code. In addition to using descriptive and intuitive names, you’ll want to ensure you don’t repeat terminology either. Take the following, for example:

module Products
  module ProductCategories
    class CategoryQuery
      CATEGORY_ORDER = :desc
      CATEGORY_LIMIT = 15

      def initialize product_model, category_order: CATEGORY_ORDER, category_limit: CATEGORY_LIMIT
        @product_model = product_model
        @category_order = category_order
        @category_limit = category_limit
      end

      def to_query = product_model.order(category_order).limit(category_limit)

      private

      attr_reader :product_model, :category_order, :category_limit
    end
  end
end

Notice how the above example violates all of the following:

  • Use of product and category is repeated multiple time within the module namespace and even the query object itself.

  • The constants unnecessarily repeat the class name in which they are defined.

  • The constructor unnecessarily repeats the use of the product and category prefixes for all parameters.

  • The public #to_query method unnecessarily repeats the object name for which it belongs and poorly defines the type of object answered back when messaged.

As you have probably concluded by now, the design of the namespace, object, methods, and related parameters is too verbose. There is no need to hit the reader over the head with redundant terminology. Instead we can simplify the above by leveraging the power of the namespaces in which the implementation is defined. Here’s a better way to write the above without sacrificing usability or readability:

module Products
  module Categories
    class Query
      ORDER = :desc
      LIMIT = 15

      def initialize order: ORDER, limit: LIMIT
        @order = order
        @limit = limit
      end

      def call(model = Category) = model.order(order).limit(limit)

      private

      attr_reader :order, :limit
    end
  end
end

Notice how the above eliminates all of the previous redundant terminology and greatly decreases the verbosity and noise within the implementation. Not only do we have a short and concise namespace for which to add and organize future objects but we also have query objects and can be constructed to answer scopes which might be chain-able by downstream objects. 🎉

Rude Mutation

A rude mutation occurs when you pass a value and mutate it as a side effect. Consider the following situation:

def yellize(text) = "#{text.upcase!}!"

message = "Hello"
yellize message  # "HELLO!"
message          # "HELLO!"

What’s surprising with the above is your message was mutated into a transformed result without any inclination it’d do this. One convention is to have a #yellize! method which indicates that there are side effects when sending the #yellize! message (i.e. mutation or exception). Using a bang-suffixed method is definitely one way to solve this problem but I’d want to ask is: Do you need the mutation in the first place? Don’t get me wrong, mutations can improve performance in certian situations but shouldn’t be the first thing you reach for. In that case, we could rewrite the implementation to be kinder to fellow engineers as follows:

def yellize(text) = "#{text.upcase}!"

message = "Hello"
yellize message  # "HELLO!"
message          # "Hello"

The above is better because there are no surprising side effects. We see our message remains unmolested and we still get a new, but transformed string, we can use for further processing. Should you need more examples of rude mutations, you should read Tim Riley’s Salubrious Ruby on this subject.

Subclassed Primitives

Primitives are objects like String, Array, Hash, etc. It can be tempting to implement an object by subclassing one of these primitives and adding your own custom behavior. For example, let’s make a subclass of Array:

class Example < Array
end

Example.new.class                  # Example
Example.new.to_a.class             # Array
(Example.new + Example.new).class  # Array

Notice how the Example instances start out reasonable enough but then take a surprising turn. This is because core primitives are implemented in C for performance reasons where the return values of several methods, like #+, are hard coded to answer the super type. This is why we get an Array instead of Example. This can lead to subtle and surprising behavior. Even structs don’t want to be subclassed.

You can avoid hard to debug behavior, like this, by using composition instead of inheritance:

class Example
  include Enumerable

  def initialize(*arguments)
    @list = Array arguments
  end

  def each(*arguments, &block) = list.each(*arguments, &block)

  private

  attr_reader :list
end

example = Example.new 1, 1, 2
example.map { |element| element * 10}  # [10, 10, 20]
example.min                            # 1
example.max                            # 2
example.tally                          # {1 => 2, 2 => 1}

With the above, we get all the benefits of Enumerable — which is generally what you want — by implementing a single method: #each. Delegation by extending Forwardable, for example, is another viable option.

Even the object size is reduced:

Array.new.methods.size   # 191
Enumerable.methods.size  # 109

That’s a savings of 82 methods you most likely don’t need either. So, yeah, save yourself some heartache, use composition, and keep your objects simple.

Conclusion

May the above improve your code quality and make you a better engineer!