How I split a fat rails model

Splitting a Ruby on a Rail

Background

Raise.com is built on top of Spree.  We have in-lined spree, and have decorators
on the models to beat the band.  We want to move away from Spree altogeher.  
This involves taking the models from Spree, moving them to the Raise
base app.  This merge has the potential to create lots of big classes.  Big classes 
are difficult to maintain.  One of the first steps towards returning classes/models to 
being "right sized." is splitting the business logic and the persistence.

What I did

I merged all the classes and decorators together.  Then I extracted the business 
logic into a module.  

There are a number of ways to approach splitting the code.  I chose the model & module
method for this experiment.  Next time I might try model, poro and delegate, or other.

For the purposes of this demo, lets assume the following class:

```ruby
# /app/models/user.rb
class User
  scope :active, { where(active: true) }
  has_many :orders
  validation :name, :presence => true

  def total_amount_for_orders
    orders.map(&:amount).inject(:+)
  end

  after_create :Send_confirmation

  private

  def send_confirmation
    UserEmail.confirmation(self).deliver
  end
end
```

======
Step 1:  Split the model
======

The model can be split into two layers:
- a class with all persistence and orm related methods
- a module with all business logic 

The class should contain:
- association declaration statements
- validation
- scopes 
- attr_accessible, et al, that have to do with persistence

The module should contain:
- the #included hook
- attr_writer/reader/accessor
- InstanceMethods module
- ClassMethods module
- methods that make calls to underlying associations
- methods that contain other class calls, these need to be teased out

With our demo we should have two files that look something like:

```ruby
# /app/model/user.rb
class User
  scope :active, { where(active: true) }
  has_many :orders
  validation :name, :presence => true
end
```

```ruby
# /app/model/user_decorator_logic.rb
module UserDecorator
  module Logic

    def self.included(klass)
      klass.send :include, InstanceMethods
      klass.send :extend, ClassMethods
    end

    module InstanceMethods
      after_create :send_confirmation

      def total_amount_for_orders
        orders.map(&:amount).inject(:+)
      end

    private

      def send_confirmation
      UserEmail.confirmation(self).deliver
      end
    end

    module ClassMethods
    end
  end
end
```

If you have existing tests, copy the existing test spec file over to a new
one for the business logic only, e.g.: 
  /spec/mode/user_decorator_logic_spec.rb
Otherwise create two specs
integration only: 
  /spec/mode/user_spec.rb
business logic only: 
  /spec/models/user_decorator_logic_spec.rb

======
the business logic file
======

In the logic spec, remove anything that has to do with persistence:
- verifying associations
- building associations
- validations
- scopes

At the top of the spec remove all "require" statements.  There should be 3
requires:

```ruby
  require "rpsec"
  require "stub_sync"
  require "[the logic module"]
```

Check if the class-under-test exists.  If it doesn't create a class that only
includes your business logic model.

The head of your logic file should look something like:


```ruby
# /spec/models/user_decorator_logic_spec.
require "rspec"
require_relative "../stub_sync"
require_relative "relative/path/to/decorator"

unless defined?(User)
  class User
    include UserDecoratorLogic
  end
end
```

If you are using an existing test spec, run the tests.  You should have broken tests.
This is your hint that its type to go and stub your associations, etc, appropriately.

If this is a fresh spec, you are now free to build out your test case and specs.  But
please consider the following guidelines.


```ruby
describe User do
  # tests go here
end
```

Time to stub class under test...errr stub something under tests.

We are splitting the business layer and the persistence layer.  This is no
easy task.

While it is never a good idea to stub the class under test, here is my argument
for doing just that, errr... kinda, and using stub sync:

  Ruby modules are generally used in two capacities: namespacing, creating
  libraries of reusable methods.  I add to this "business logic for models, not
  necessarily intended for reuse."  In this capacity, I would argue that we
  are not "stubbing" class under test.  I would say we are testing the business
  logic currently wrapped in a mock model, and stubbing the persistence layer 
  that will be there at some point before deploy.  
    We use StubSync to identify which methods are currently not aviable to the 
  class being test.  During the full suite test, any methods that StubSync flags need
  to be implemented before shipping.

Where were we...back to stabbing... err.. stubbing, so, yeah,  we stub persistence:

```ruby
describe User do
  describe "#total_amount_for_orders" do
    it "has a total amount of zero" do
      subject.stub(:orders => [])
      expect(subject.total_amount_for_orders).to be zero
    end

    it "has a total amount greater than zero" do
      orders = [ double(amount: 1), double(amount: 2) ]
      subject.stub(:orders => orders)
      expect(subject.total_amount_for_orders).to be eq(3)
    end
  end
end
```

What to note here:
- We stub the association: no database call made!
- StubSync will alert us if the association doesn't exist, during full
  integration test
- Only the business logic is tested
- Super fast, its all in the memory!


======
the integration test with persistence
======

You can easily adapt these tests to an integrated spec, and what not.  Depending
on your practices, or your groups accepted practices, your integration test
might look something like:

```ruby
describe User do
  subject { FactoryGirl.create :user }

  describe "#total_amount_for_orders" do

    it { should have_many(:orders) } # Sanity

    context "without orders" do
      it "has a total amount of zero" do
        expect(subject.total_amount_for_orders).to be zero
      end
    end

    context "with orders" do
      before do
        FactoryGirl.create(:order, amount: 1, user_id: subject.id)
        FactoryGirl.create(:order, amount: 2, user_id: subject.id)
        subject.orders(true)
      end

      it "has a total amount greater than zero" do
        subject.stub(:orders => orders)
        expect(subject.total_amount_for_orders).to be eq(3)
      end
    end
  end
end
```

Using the guidelines above, we can harness the power of the rapid feedback loop.
This loop will allow us to use tdd/bdd as it was intended for the purposes 
of designing and guiding system logic and pushing the details further downstream.

The full integration and acceptance suite should be run before going live, and can be done so as part of a CI server or a github hook.  The logic-only testing, doubles as the contract you built the for.  While the integration tests will show you what you are really receiving.

====
Notes from doing it live:
====

When you split business logic and persistence, violations of SOLID principles,
best practices, and code smells are immediately in your face.

If you cannot test the business logic without persistence or you cannot test 
without introducing other models into the test, you may have problems.

Just some examples:

-Example 1: Attribute envy, meet scope-

```ruby
class User
  def complete_orders
    orders.select{ |o| o.state == "complete" && o.payment_state == "paid" }
  end
end
```

```ruby
class User
  def complete_orders
    orders.paid
  end
end
```

Creating a test for this involved creating an array of doubles that set state,
and payment_state.  Kinda lame right? Well, this is the perfect opportunity to
refactor an attribute envy smell into a order scope.  Funny enough, someone
beat me to the punch and created a :paid scope that contained list slogic

- Example 2: Dependency meet [something]-

```ruby
def enough_sold_listings?
  Order.number_sold_by(self) >= 4
end
```

Consider ...?

```ruby
def enough_sold_listings?(num = Order.number_sold_by(self))
  num >= 4
end
```

Okay okay okay this is ugly and not a good solution.  What would be good to address is how to
tease out the Order dependency.

- Example 3: Private Methods, strutinize them -

The following is called before the validation:

```ruby
def reformat
  self.phone.gsub!(/\D/, "") if self.phone
end
```

A possible solution might be to reformat on assignment

```ruby
def phone=(val)
  write_attribute :phone, val.gsub(/\D/,'')
end
```

I think this a nice shift not only to unSpree User but to guideline further development. Congrats.