
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
- Flat Modules
- Global Variables
- Insecure Constantization
- Insecure Open
- Insecure Templates
- Jumbled Message Chains
- Lazy Operator
- Misused Casting
- Misused Endless Methods
- Misused Method Parameters
- Monkey Patches
- Multi-Object Files
- Multiple Inheritance
- Nested Classes
- Nonclasses
- Numbered Parameters
- Obvious Comments
- Privacy Invasion
- Redundant Naming
- Rude Mutation
- Subclassed Primitives
- Conclusion
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 asnil
because once we start withnil
, there is nothing stoppingnil
propagating through the entire message chain? -
Is it because we are uncertain whether
author
will be present or maybe onlyaddress
could potentially benil
andauthor
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
andzip
). 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
foraddress
and thenzip
,address
could answer back a null object that acts like anaddress
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 thenil
tends to leak into more code that needs to deal with anil
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 thenaddress
to pluck out the value ofzip
. We could teachauthor
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 ofNoMethodError
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 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):
-
Positional (optional)
-
Positional (required)
-
Positional (single splat)
-
Keyword (optional)
-
Keyword (required)
-
Keyword (double splat)
-
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 andExample
class are defined in ademo.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!