Design Patterns in the Wild: Null Object
I knocked out a pretty decent refactoring of some of the internals of Factory Girl this past weekend. In one of my commits, I used the Null Object pattern to simplify some conditional logic that was spread across a class.
What's the Null Object Pattern?
The Null Object pattern describes the use of an object to define the concept of "null" behavior. Typically, a null object will implement a similar interface to a similar object but not actually do anything.
In the instance of Factory Girl, there's a FactoryGirl::Factory object and a FactoryGirl::NullFactory object; both have a common interface by responding to the instance methods defined_traits, callbacks, attributes, compile, default_strategy, and class_name. These methods are the core of a FactoryGirl::Factory and it's important that the methods exist on the NullFactory (we'll be calling these methods in FactoryGirl::Factory).
In Factory Girl, the Factory object deals with taking a handful of declared attributes and running it, resulting in an instance of a class with values assigned. Factory Girl supports the concept of parent factories; attributes, callbacks, and other features get inherited, but a parent isn't required. Here's the code before the change; as you can see, there's a ton of checking to see if the parent exists.
module FactoryGirl class Factory def default_strategy @default_strategy || (parent && parent.default_strategy) || :create end def compile if parent parent.defined_traits.each {|trait| define_trait(trait) } parent.compile end attribute_list.ensure_compiled end protected def class_name @class_name || (parent && parent.class_name) || name end def attributes compile AttributeList.new(@name).tap do |list| traits.each do |trait| list.apply_attribute_list(trait.attributes) end list.apply_attribute_list(attribute_list) list.apply_attribute_list(parent.attributes) if parent end end def callbacks [traits.map(&:callbacks), @definition.callbacks].tap do |result| result.unshift(*parent.callbacks) if parent end.flatten end private def parent return unless @parent FactoryGirl.factory_by_name(@parent) end end end
Not only does it add extra lines of code (and every line of code is a liability), but it also forces other developers reading the code to remember if a parent exists. This context-switching across five different methods makes it hard to remember what the actual behavior of each method is doing because certain things may or may not be executed.
To use the pattern, all I did was create a NullFactory object and implement the interface I knew I needed to get rid of all the conditionals. For each method, I returned a "sensible" result; nil for class_name, default_strategy, and compile, and I delegated the remaining few methods (defined_traits, callbacks, and attributes) to definition.
module FactoryGirl class NullFactory attr_reader :definition def initialize @definition = Definition.new end delegate :defined_traits, :callbacks, :attributes, :to => :definition def compile; end def default_strategy; end def class_name; end end end
Testing is pretty straightforward since the behavior is straightforward.
describe FactoryGirl::NullFactory do it { should delegate(:defined_traits).to(:definition) } it { should delegate(:callbacks).to(:definition) } it { should delegate(:attributes).to(:definition) } its(:compile) { should be_nil } its(:default_strategy) { should be_nil } its(:class_name) { should be_nil } end
Now, the private instance method will always return something that behaves like a FactoryGirl::Factory. Perfect.
def parent if @parent # the only conditional to determine if a parent exists FactoryGirl.factory_by_name(@parent) else NullFactory.new end end
Here are those methods after introducing the Null Object pattern.
module FactoryGirl class Factory def default_strategy @default_strategy || parent.default_strategy || :create end def compile parent.defined_traits.each {|trait| define_trait(trait) } parent.compile attribute_list.ensure_compiled end protected def class_name @class_name || parent.class_name || name end def attributes compile AttributeList.new(@name).tap do |list| traits.each do |trait| list.apply_attribute_list(trait.attributes) end list.apply_attribute_list(attribute_list) list.apply_attribute_list(parent.attributes) end end def callbacks [parent.callbacks, traits.map(&:callbacks), @definition.callbacks].flatten end private def parent if @parent FactoryGirl.factory_by_name(@parent) else NullFactory.new end end end end
The commit in Factory Girl can be found here. As you can see, the logic is simplified greatly across all the methods because there's no more conditional checking. The developer reading this code doesn't have to care if a parent is assigned or not because he can be sure that the parent, regardless of what it is, will behave in the correct manner when that method is executed.
A couple of months ago, Gabe and I implemented the same pattern in Kumade by introducing a NoopPackager.
Have you used the Null Object pattern recently? If you haven't, your code is probably ripe for some Null Object pattern disruption!