Extract Workflow Objects From Fat Models

If you’re wondering why I’ve got fat-model obsession, there’s one simple reason - CodeClimate is a highly addictive and enjoyable way to learn just how much your code sucks while telling you where exactly you should be focussing your improvements and once you get started, you just can’t stop. Basically, refactoring extensively so that all your classes are tiny and focussed, and the accompanying feeling of freedom and fluidity that comes from this drives your code quality higher and higher.

People have talked at length about the benefits of SRP but all that reading hadn’t prepared me for the actual experience of whittling classes down to doing just one thing.

• Firstly, you can write really focussed tesets. Now more than half of the files in my models directory have nothing really to do with Rails. As POROs, testing them becomes super fast.
• The elimination of churn from the classes is the biggest source of relief for me. Once the class is specced and written, it almost never changes and this gives me a lot more confidence that changes I make in one part of the code won’t have unintended consequences elsewhere.

The whole experience has been so fundamentally mind altering that I would recommend everyone try out CodeClimate.

During this process, naturally, I learned a lot about keeping models focussed on one thing. CodeClimate’s biggest problem always has been with the models that have an embedded workflow in them, and I figured it might be worthwhile to try and extract the workflow into an object all its own. Here’s my story.

To start with, I have the following scenario. The model in question is the Quotation model, which handles the creation of quotations to be sent to clients. The quotations go through various stages, first being approved or rejected by an admin, then being sent to the client and finally being paid. The original model took care of defining and persisting the attributes. In addition it could answer various questions about what all could be done with the quotation in whatever state it happened to be in, as well as handling all state transitions (including sending email and so on).

Broadly, the class looked like this

	class Quotation
	include DataMapper::Resource
	# ...snip...
	property :quoted_kms, Float
	property :quoted_per_km, Float
	property :quoted_price, Float
	property :start_date, Date, :required => true
	property :end_date, Date, :required => true
	# ...snip...
	def unconfirmed?
	[:mildly_interested, :negotiating].include?(quality)
	end
	def expirable?
	true if self.start_date <= Date.today
	end
	def sendable?
	(approved? || sent?) && !intended_trip.user.email.blank? && !expirable?
	end
	def approvable?
	set_end_date
	(start_date && end_date) && !sent? && !unconfirmed? && !approved? && !expirable?
	end
	#... loads of small functions that check whats possible to do with the Quotation in its current state
	workflow do
	state :created do
	event :approve, :transitions_to => :approved, :if => Proc.new{|t| t.approvable? }
	event :reject, :transitions_to => :rejected, :if => Proc.new{|t| t.rejectable? }
	event :hold, :transitions_to => :on_hold, :if => Proc.new{|t| t.on_holdable? }
	event :expire, :transitions_to => :expired, :if => Proc.new{|t| t.expirable? }
	event :callback, :transitions_to => :callback, :if => Proc.new{|t| t.callbackable? }
	end
	state :approved do
	event :send_quotation, :transitions_to => :sent, :if => Proc.new{|t| t.sendable? }
	event :reject, :transitions_to => :rejected, :if => Proc.new{|t| t.rejectable? }
	event :hold, :transitions_to => :on_hold, :if => Proc.new{|t| t.on_holdable? }
	event :pay, :transitions_to => :paid, :if => Proc.new{|t| t.payable? }
	event :expire, :transitions_to => :expired, :if => Proc.new{|t| t.expirable? }
	end
	#... snip ...
	end
	# ..snip ..
	end
	def send_quotation(doer, params = {})
	valid?
	UserMailer.quotation(self).deliver
	self.sent_at = DateTime.now
	Transaction.all(:amount.lte => 0, :quotation => self).destroy
	t = Transaction.create(:amount => -quoted_price, :date => Date.today, :user => self.intended_trip.user, :created_by => doer, :comment => "#{doer.email} sent quotation", :quotation => self)
	end
	def pay(doer, params = {})
	t = Transaction.create(:amount => quoted_price, :date => Date.today, :user => self.intended_trip.user, :created_by => doer, :comment => "#{doer.email} marked as 'Paid'", :quotation => self)
	end
	# some more actions to be done on state transition
	# loads of other methods to do with the Quotation proper
	end

view raw quotation.rb hosted with ❤ by GitHub

The Quotation class is the God class of this application. It has a trip associated with it, a duration, is pooled with other Quotations in shared cabs, it has several fields which represent various orthagonal states (lead quality is one state, the workflow which leads to payment and closure is another), it shows up in the users account and so on. The whole class is very complex and was begging to be ripped apart, scoring as it did, a big fat F on CodeClimate.

So, among other things (such as creating separate scope and search classes), I started to extract a workflow class from this class using the dependency injection technique I talked about in my previous post. As the model currently is, there is a very tight coupling between the Quotation class and its workflow. It is not also possible to have more than one workflow field in the model. In addition, there is no way to have separate workflows based on say, the user associated with the quotation (i.e. different criteria and workflows for different classes of users like guest user, trial user, premium user, etc.) I am happy to report that ripping apart the class and using dependency injection has solved all these problems.

So, the first thing I did was to extract a QuotationStatusPolicy class which would handle all the questions about whether a given quotation was approvable, sendable and so on. i.e.

	class QuotationStatusPolicy
	def initialize(quotation)
	@quotation = quotation
	end
	# STATUSES
	def unconfirmed?
	[:mildly_interested, :negotiating].include?(quality)
	end
	def expirable?
	true if self.start_date <= Date.today
	end
	def sendable?
	(approved? || sent?) && !intended_trip.user.email.blank? && !expirable?
	end
	def approvable?
	set_end_date
	(start_date && end_date) && !sent? && !unconfirmed? && !approved? && !expirable?
	end
	def rejectable?
	!sent? && !rejected? && !expirable?
	end
	def poolable?
	(approved? || sent? || paid?) && !expirable?
	end
	def on_holdable?
	(!sent? && !unconfirmed?) && !expirable?
	end
	def payable?
	true unless (approvable? || expirable?)
	end
	def callbackable?
	true unless approved? || sent? || rejected? || paid?
	end
	def method_missing(name, *args)
	@quotation.send(name, *args)
	end
	end

view raw quotation_policy_status.rb hosted with ❤ by GitHub

Using this method, it becomes trivially simple to use a different status policy class for various classes of users.

Now, we need to rip out the heart of the beast, the actual workflow itself. This is done as follows

	class QuotationWorkflow
	def initialize(object)
	@object = object
	end
	include Workflow
	workflow do
	state :created do
	event :approve, :transitions_to => :approved, :if => Proc.new{|t| t.approvable? }
	event :reject, :transitions_to => :rejected, :if => Proc.new{|t| t.rejectable? }
	event :hold, :transitions_to => :on_hold, :if => Proc.new{|t| t.on_holdable? }
	event :expire, :transitions_to => :expired, :if => Proc.new{|t| t.expirable? }
	event :callback, :transitions_to => :callback, :if => Proc.new{|t| t.callbackable? }
	end
	state :approved do
	event :send_quotation, :transitions_to => :sent, :if => Proc.new{|t| t.sendable? }
	event :reject, :transitions_to => :rejected, :if => Proc.new{|t| t.rejectable? }
	event :hold, :transitions_to => :on_hold, :if => Proc.new{|t| t.on_holdable? }
	event :pay, :transitions_to => :paid, :if => Proc.new{|t| t.payable? }
	event :expire, :transitions_to => :expired, :if => Proc.new{|t| t.expirable? }
	end
	state :sent do
	event :send_quotation, :transitions_to => :sent, :if => Proc.new{|t| t.sendable? }
	event :pay, :transitions_to => :paid, :if => Proc.new{|t| t.payable? }
	event :expire, :transitions_to => :expired, :if => Proc.new{|t| t.expirable? }
	end
	state :on_hold do
	event :approve, :transitions_to => :approved, :if => Proc.new{|t| t.approvable? }
	event :reject, :transitions_to => :rejected, :if => Proc.new{|t| t.rejectable? }
	event :expire, :transitions_to => :expired, :if => Proc.new{|t| t.expirable? }
	event :callback, :transitions_to => :callback, :if => Proc.new{|t| t.callbackable? }
	end
	state :rejected do
	event :approve, :transitions_to => :approved, :if => Proc.new{|t| t.approvable? }
	event :hold, :transitions_to => :on_hold, :if => Proc.new{|t| t.on_holdable? }
	event :expire, :transitions_to => :expired, :if => Proc.new{|t| t.expirable? }
	end
	state :paid do
	event :expire, :transitions_to => :expired, :if => Proc.new{|t| t.expirable? }
	end
	state :expired
	state :callback do
	event :approve, :transitions_to => :approved, :if => Proc.new{|t| t.approvable? }
	event :reject, :transitions_to => :rejected, :if => Proc.new{|t| t.rejectable? }
	event :hold, :transitions_to => :on_hold, :if => Proc.new{|t| t.on_holdable? }
	event :callback, :transitions_to => :callback, :if => Proc.new{|t| t.callbackable? }
	end
	before_transition do |from, to, event, *event_args|
	self.callback_at = nil unless event == :callback
	end
	on_transition do |from, to, event, *event_args|
	valid?
	doer = event_args[0]
	halt "Not Authorised" unless Ability.new(doer).can?(event, self)
	al = ActivityLog.new(:from => from, :to => to, :event => event, :user => self.intended_trip.user,
	:staff => (doer if doer.class == Staff), :parent_model => self.class.to_s, :parent_id => self.id)
	al.save
	end
	end
	def send_quotation(doer, params = {})
	valid?
	UserMailer.quotation(self).deliver
	self.sent_at = DateTime.now
	Transaction.all(:amount.lte => 0, :quotation => @object).destroy
	t = Transaction.create(:amount => -quoted_price, :date => Date.today, :user => intended_trip.user, :created_by => doer, :comment => "#{doer.email} sent quotation", :quotation => self)
	end
	def pay(doer, params = {})
	t = Transaction.create(:amount => quoted_price, :date => Date.today, :user => self.intended_trip.user, :created_by => doer, :comment => "#{doer.email} marked as 'Paid'", :quotation => self)
	end
	def callback(user, params)
	self.callback_at = params[:callback_at] unless attribute_dirty?(:callback_at)
	halt("require callback_at time in order to mark as calback") unless self.callback_at
	end
	def expire(user, params = {})
	@object.quality = :expired
	end
	def load_workflow_state
	@object.workflow_state
	end
	def persist_workflow_state(new_value)
	return if halted?
	@object.workflow_state = new_value
	@object.save!
	end
	def method_missing(name, *args)
	@object.send(name, *args)
	end
	end

view raw quotation_workflow.rb hosted with ❤ by GitHub

The last thing thats left is to wire up these new classes into the Quotation class. i.e.

	class Quotation
	# ...snip...
	# methods to be forwarded to the status policy class
	def self.status_policy_methods
	[:unconfirmed?, :expirable?, :approvable?, :rejectable?, :poolable?, :on_holdable?, :payable?, :callbackable?, :sendable?]
	end
	# methods to be forwarded to workflow class
	def self.workflow_methods
	QuotationWorkflow.instance_methods(false) - [:method_missing] + [:has_events?, :has_only_events?]
	end
	# the class to be used for the status policy
	def status_policy(policy_class = QuotationStatusPolicy)
	policy_class.new(self)
	end
	def workflow(workflow_class = QuotationWorkflow)
	workflow_class.new(self)
	end
	extend Forwardable
	def_delegators :status_policy, *(Quotation.status_policy_methods)
	def_delegators :workflow, *(Quotation.workflow_methods)
	# ...snip ...
	end

view raw quotation.rb hosted with ❤ by GitHub

All tests passing? We’re good to go!