Refactor

coldnebo edited this page Jun 14, 2012 · 8 revisions
Clone this wiki locally

Condi as an aid to refactoring

So, what's interesting about Condi is that it allows you to gradually move towards "doing the right thing". What is the "right thing" where business logic is concerned? Well, I mentioned previously, it's more pragmatic, even if not technically correct to place business logic in the controller via Condi.

The Progression

Say you inherit horrible business logic in your view like this:

app/views/store/checkout.html.erb:

<% form_for @cart do |f| %>
  <% if @day.monday? || @moon.full? && @chicken.state == :dancing || @magic == 21 %>
    <%= f.radio_button("cart", "shipping", "free_ground") %>
  <% end %>
<% end %> 

Move conditionals out of the view

What is that horrible conditional? How can you possibly start refactoring this? Well, take it one step at a time... First, move the horrible conditional out of the view.

app/controllers/store_controller.rb:

class StoreController
  include Condi
  def checkout
    @day = Date.new
    @moon = Moon.current
    @chicken = Chicken.find(3)
    @magic = MagicFactory.generate_magic!
    predicate(:show_free_shipping?) { @day.monday? || @moon.full? && @chicken.state == :dancing || @magic == 21 }
  end
end

app/views/store/checkout.html.erb:

<% form_for @cart do |f| %>
  <% if show_free_shipping? %>
    <%= f.radio_button("cart", "shipping", "free_ground") %>
  <% end %>
<% end %> 

Remove unnecessary instance vars

Next, you can get rid of the nasty instance vars... you don't need them with a closure.

app/controllers/store_controller.rb:

class StoreController
  include Condi
  def checkout
    day = Date.new
    moon = Moon.current
    chicken = Chicken.find(3)
    magic = MagicFactory.generate_magic!
    predicate(:show_free_shipping?) { day.monday? || moon.full? && chicken.state == :dancing || magic == 21 }
  end
end

move state and predicates to before_filters where applicable

Say a second action checkout_continued models a multi-step checkout flow. We might allow the user to check free shipping in the first action and then only see a locked selection in the second action. But both actions need our predicate and resulting state. So move the state and the predicate to a before_filter!

app/controllers/store_controller.rb:

class StoreController
  include Condi
  before_filter :check_free_shipping, :only => [:checkout, :checkout_continued]

  def check_free_shipping
    day = Date.new
    moon = Moon.current
    chicken = Chicken.find(3)
    magic = MagicFactory.generate_magic!
    predicate(:show_free_shipping?) { day.monday? || moon.full? && chicken.state == :dancing || magic == 21 }      
  end

  def checkout  
  end

  def checkout_continued
  end
end

Refactor to business rules!

From here it's not hard to refactor into an actual ShippingRules object and then keep the state in one place.

Summary

It might not seem we did anything very useful because this example is fairly straightforward. But imagine we inherit a view where there are a bunch of conditional if, cases, etc. and multiple existing actions that use various bits of state via controller instance vars in different contexts (post, get, xhr, etc.). Now you have to tease all that mess apart without breaking existing behavior. Condi makes it easier to divide and conquer this hairball until you get to a point where it is easy to see which rules objects you'll need and which action contexts rely on the information.

Good luck!