Background
In the Learn app, we need to accept purchaseable items (books, screencasts,
workshops, plans, and so on) as a parameter in several places. Because we’re
using Rails’ polymorphic_path
under the hood, these parameters come in
based on the resource name, such as book_id
or workshop_id
. However, we
need to treat them all as “purchaseables” when users are making purchases, so
we need a way to find one of several possible models from one of several
possible parameter names in PurchasesController
and a few other places.
The logic for finding these purchaseables was previously on
ApplicationController
:
def requested_purchaseable
if product_param
Product.find(product_param)
elsif params[:individual_plan_id]
IndividualPlan.where(sku: params[:individual_plan_id]).first
elsif params[:team_plan_id]
TeamPlan.where(sku: params[:team_plan_id]).first
elsif params[:section_id]
Section.find(params[:section_id])
else
raise "Could not find a purchaseable object from given params: #{params}"
end
end
def product_param
params[:product_id] ||
params[:screencast_id] ||
params[:book_id] ||
params[:show_id]
end
This method was problematic in a few ways:
ApplicationController
is a typical junk drawer, and it’s unwise to feed it.- The method grew in complexity as we added more purchaseables to the application.
- Common problems, such as raising exceptions for bad IDs, could not be implemented in a generic fashion.
- Testing
ApplicationController
methods is awkward. - Testing the current implementation of the method was repetitious.
While fixing a bug in this method, I decided to roll up my sleeves and use a few new objects to clean up this mess.
The Fix
The new method in ApplicationController
now simply composes and delegates to
a new object I created:
def requested_purchaseable
PolymorphicFinder.
finding(Section, :id, [:section_id]).
finding(TeamPlan, :sku, [:team_plan_id]).
finding(IndividualPlan, :sku, [:individual_plan_id]).
finding(Product, :id, [:product_id, :screencast_id, :book_id, :show_id]).
find(params)
end
The class composed and delegates to two small, private classes:
# Finds one of several possible polymorphic members from params based on a list
# of relations to look in and attributes to look for.
#
# Each polymorphic member will be tried in turn. If an ID is present that
# doesn't correspond to an existing row, or if none of the possible IDs are
# present in the params, an exception will be raised.
class PolymorphicFinder
def initialize(finder)
@finder = finder
end
def self.finding(*args)
new(NullFinder.new).finding(*args)
end
def finding(relation, attribute, param_names)
new_finder = param_names.inject(@finder) do |fallback, param_name|
Finder.new(relation, attribute, param_name, fallback)
end
self.class.new(new_finder)
end
def find(params)
@finder.find(params)
end
private
class Finder
def initialize(relation, attribute, param_name, fallback)
@relation = relation
@attribute = attribute
@param_name = param_name
@fallback = fallback
end
def find(params)
if id = params[@param_name]
@relation.where(@attribute => id).first!
else
@fallback.find(params)
end
end
end
class NullFinder
def find(params)
raise(
ActiveRecord::RecordNotFound,
"Can't find a polymorphic record without an ID: #{params.inspect}"
)
end
end
private_constant :Finder, :NullFinder
end
The new class was much simpler to test.
It’s also easy to add new purchaseable types without introducing unnecessary complexity or risking regressions.
The explanation
The solution uses a number of constructs and design patterns, and may be a little tricky for those unfamiliar with them:
- The Builder pattern
- The Decorator pattern
- The Chain of Responsibility pattern
- The Null Object pattern
- Recursion, using a
fold
/inject
- Object composition
- Immutable objects
It works like this:
- The
PolymorphicFinder
class acts as a Builder for theFinder
interface. It acceptsinitialize
arguments forFinder
, and encapsulates the logic of chaining them together. - The
finding
instance method ofPolymorphicFinder
usesinject
to recursively build a chain ofFinder
instances for each of theparam_names
that theFinder
should look for. - Each
Finder
in the chain accepts a fallback. In the event that theFinder
doesn’t know how to find anything from the given params, it delegates to its fallback. This forms a Chain of Responsibility. - The first
Finder
is initialized with aNullFinder
, which forms the last resort of the Chain of Responsibility. In the event that everyFinder
instance delegates to its fallback, it will delegate to theNullFinder
, which will raise a useful error of the correctException
subclass. - The
PolymorphicFinder
class also acts as a Decorator for theFinder
interface. Once the Builder interaction is complete, you can callfind
on thePolymorphicFinder
(just as you would for a regularFinder
) and it will delegate to the firstFinder
in its chain.
Benefits
- The new code replaces conditional logic and special cases with polymorphism, making it easier to change.
- The usage (in
ApplicationController
) is much easier to read and modify. - Adding or changing finders is less likely to introduce regressions, since common issues like blank or unknown IDs are handled generically.
- The code avoids possible state vs identity bugs by avoiding mutation.
Drawbacks
- The new code is larger, both in terms of lines of code and overall complexity by any measure.
- It uses a large number of design patterns that will be confusing to those that are unfamiliar with them, or to those that fail to recognize them quickly.
- It introduces new words into the application vocabulary. Although naming things can reveal their intent, too many names can cause vocabulary overload and make it difficult for readers to hold the problem in their head long enough to understand it.
Conclusion
In summary, using the new code is easier, but understanding the details may be
harder. Although each piece is less complex, the big picture is more complex.
This means that you can understand ApplicationController
without knowing how
it works, but knowing how the whole thing fits together will take longer.
The heavy use of design patterns will make the code very easy to read at a macro level when the patterns are recognized, but will read more like a recursive puzzle when the patterns aren’t clear.
Overall, the ease of use and the improve resilience to bugs made me decide to keep this refactoring despite its overall complexity.
Also available in 3D
Okay, maybe not 3D, but Ben and I also discussed this in a video on our show, the Weekly Iteration, available to Upcase subscribers.
What’s next
If you found this useful, you might also enjoy:
- Ruby Science, for more information on design patterns and refactoring.
- Discussion of state vs identity in the Clojure documentation.
- The full example on GitHub.