Monday, April 9, 2012

Refactoring Workflows to Chain of Actions

Everything we do in life is a series of actions. Just to get to work I need to wake up, eat something, brush my teeth and drive to work. Or when the data is sent to the server your code has to validate the user input, it has to create a new object with the attributes, this new data has to be saved and a response data needs to be generated in JSON format if your request happens to be an Ajax request.

When I write code for a series of tasks I start out with a "coordinator" object that has several private methods internally. The main and public method orchestrates the calls for the private methods. The tests around this object start out pretty nice, but as the complexity grows I need to stub more and more external objects. The complexity of the tests are soon becoming indicators of the worsening design and I need to start pulling out objects before the whole thing turns into an iceberg class.

The example I use in this writing is a simple one: The "Dude" wants to go to the park to enjoy the nice weather. He has to go through several steps to get there:

  1. Leave the house
  2. Close the door
  3. If he has a car - Jump in the car
  4. If he has a car - Drive to the park
  5. If he has a car - Park the car
  6. If he has NO car - Jump on the bike
  7. If he has NO car - Ride to the park
  8. If he has NO car - Park the bike
  9. Enter the park
  10. Take a walk

All my examples are in CoffeeScript. I use CS for brevity and for its concise format.

In my example the coordinator object is called "GoesToThePark". It interacts with the House, Car, Bicycle and Park models like this:

And all this described in CoffeeScript:

House = {
  leave: (dude) ->
    'leaves the house'
  closeTheDoor: (dude) ->
    'closes the door'
}
Car = {
  jumpIn: (dude) ->
    'jumps in the car'
  driveToThePark: (dude) ->
    'drives to the park'
  parkTheCar: (dude) ->
    'parks the car'
}
Bicycle = {
  jumpOn: (dude) ->
    'jumps on the bike'
  rideToThePark: (dude) ->
    'rides to the park'
  parkTheBike: (dude) ->
    'parks the bike'
}
Park = {
  enter: (dude) ->
    'enters the park'
}

class GoesToThePark
  constructor: ->
    @messages = []

  toEnjoyTheWeather: (dude)->
    @messages.push House.leave(dude)
    @messages.push House.closeTheDoor(dude)
    if dude.hasCar()
      @messages.push Car.jumpIn(dude)
      @messages.push Car.driveToThePark(dude)
      @messages.push Car.parkTheCar(dude)
    else
      @messages.push Bicycle.jumpOn(dude)
      @messages.push Bicycle.rideToThePark(dude)
      @messages.push Bicycle.parkTheBike(dude)
    @messages.push Park.enter(dude)

Please check out this gist to see the specs. I used mocha to test-drive my code.

It's all nice and sweet. Except we have that nasty "if statement" in the middle of the GoesToThePark#toEnjoyTheWeather method.

Whenever I see a conditional block in the middle of a function call I immediately assume the violation of the Single Responsibility Principle.
I tolerate guard conditions in methods, but that "if statement" must die.

I remembered in my early Java and C# days reading about the Chain of Responsibility design pattern. Every little command object is linked together with a linked list, the first one is called from the "coordinator" object and they each check if there is anything to do with the arguments. If there is, the action is executed and at the end of the method call the next command in the chain is being called.

I found them especially helpful in workflows similar to the example described above. The coordinator object only knows about the action objects and its only responsibility is to call the one and only method on them in order. There is no conditional in the method any more, the actions are smart enough to figure out if they have to deal with the object in the context or not.

I introduce the four new action objects:

  1. LeavesTheHouse - delegates calls to the House object
  2. DrivesToThePark - invokes the methods on the Car object if the dude has a car
  3. RidesToThePark - sends messages to the Bicycle object if the dude has no car
  4. EntersThePark - executes the enter method on the Park object
Only the DrivesToThePark and RidesTheBikeToThePark protects itself with guard conditions, their execution is dependent on the fact of the Dude having a car or not. But those are simple return statements at the very beginning of the method call.

...

LeavesTheHouse = {
  execute: (messages, dude) ->
    messages.push House.leave(dude)
    messages.push House.closeTheDoor(dude)
}

DrivesToThePark = {
  execute: (messages, dude) ->
    return unless dude.hasCar()

    messages.push Car.jumpIn(dude)
    messages.push Car.driveToThePark(dude)
    messages.push Car.parkTheCar(dude)
}

RidesToThePark = {
  execute: (messages, dude) ->
    return if dude.hasCar()

    messages.push Bicycle.jumpOn(dude)
    messages.push Bicycle.rideToThePark(dude)
    messages.push Bicycle.parkTheBike(dude)
}

EntersThePark = {
  execute: (messages, dude) ->
    messages.push Park.enter(dude)
}

class GoesToThePark
  constructor: ->
    @messages = []

  toEnjoyTheWeather: (dude)->
    for action in [LeavesTheHouse, DrivesToThePark, RidesToThePark, EntersThePark]
      do =>
        action.execute(@messages, dude)

...

You can review the entire file in this gist.

The beauty of this code lies in the toEnjoyTheWeather() method. It is simple and now it's super easy to test.


... 

  toEnjoyTheWeather: (dude)->
    for action in [LeavesTheHouse, DrivesToThePark, RidesToThePark, EntersThePark]
      do =>
        action.execute(@messages, dude)

...

In fact, I worked on a Ruby code where the coordinator object called a dozen different objects through it's private methods. Tests were brittle, I had to stare at the code to figure out why something was failing after a simple change. My specs were a clear indication that the code needed serious refactoring. I changed my code using the pattern above and I eliminated all the private methods - they became simple action objects - and testing became much simpler.

Here is what it takes to test the coordinator object's method with stubs:

should = require 'should'
sinon = require 'sinon'

...

describe 'GoesToThePark', ->

  it 'calls the actions in order', ->
    goesToThePark = new GoesToThePark
    messages = goesToThePark.messages
    dude = {}

    leavesTheHouseStub = sinon.stub(LeavesTheHouse, 'execute') \
                              .withArgs(messages, dude)
    drivesToTheParkStub = sinon.stub(DrivesToThePark, 'execute') \
                               .withArgs(messages, dude)
    ridesToTheParkStub = sinon.stub(RidesToThePark, 'execute') \
                              .withArgs(messages, dude)
    entersTheParkStub = sinon.stub(EntersThePark, 'execute') \
                             .withArgs(messages, dude)

    goesToThePark.toEnjoyTheWeather(dude)
    entersTheParkStub.called.should.be.true
    drivesToTheParkStub.called.should.be.true
    ridesToTheParkStub.called.should.be.true
    entersTheParkStub.called.should.be.true

I leave you the exercise of writing the specs with stubs for the example prior to using the action objects.

Listen to your tests, they tell you the story (or the quality) of your code. Don't be afraid of creating tiny classes or objects with only 6-10 lines of code. They are super easy to test and I consider them the building blocks of reliable and maintainable software.

Big thanks to websequencediagrams.com for their tool I used to create the sequence diagram in this blog post.

Monday, March 5, 2012

Job Change

After about a year of employment I decided to leave my employer.
We have had great times together: I worked on their software rewrite, helped them move from 23 (most of them failing specs) to some 5700 passing specs and we started on the curvy path of automated acceptance tests. I had worked with some amazing people there whom I'll miss in the future.

I wasn't looking for a new job. No, but an opportunity came up and I did not want to miss it.

A good friend of mine - Dave Speck - joined a small startup in Independence, OH late last year. I talked to him a couple of times and it seemed I could do a lot of things there. A few meetings and some beers later I made up my mind: I joined Dimple Dough in the middle of February.

I have only worked there for a little over a week but we have already done so much!
Here is one of them:

We can provide basic translation for our international clients, but some of them want to further customize it. The only way they can make translation changes is sending us what they want to change and one of our engineers has to do the updates in our database. Our customers would be happy to do it themselves, if there was a tool they could use. Hence the translator idea was born. We had a vague idea what it will look like but we did not know how the tool SHOULD exactly WORK.

We started by prototyping the tool in pure HTML with CSS and JavaScript. The benefit of doing this is the low cost of change. Imagine how far less expensive it is to modify a raw prototype than the fully functioning product. There are no domain objects, data models, data migrations to change when the client wants to tweak the preview version. It's just a dummy HTML with very simple jQuery that allows us to demonstrate it to the client who can provide us feedback well before development begins.

Once we knew our prototype was close to what we wanted to build and our customer was happy with it we sat down with our business and quality focused team members. In this "three amigos" meeting (BA, QA and Developer) we wrote scenarios in Gherkin syntax using our prototype and other documentation the team had collected by then.

It made me smile to realize that after the first three or four scenarios we were discussing edge cases nobody thought about before. The scenarios we came up with are short and are not tightly coupled to the User Interface, they explain how this new tool should behave.

I tried using Gherkin and cucumber at my previous employer, but I don't think it really caught on there. After talking with @chzy (Jeff Morgan) on a cold December morning I understood why: we used Gherkin for automated system testing and not to discover functionality with BAs and QAs prior to development.

Monday, January 23, 2012

JavaScript Testing with Mocha

JavaScript is a neat and powerful language. Sure it has its flaws, but serious software can be developed with it. The way I prefer developing JavaScript applications is by test driving the code with some kind of testing tool. And I am not thinking about hitting the browser's refresh button. No, I mean executing the specs right in the terminal.

I recently started playing with Visionmedia's mocha testing framework. The code is well maintained and the authors are responding fairly fast to pull requests or issues.
I would recommend it as an alternative to Jasmine.

This blog post will show you the first couple of steps you need to take to test drive your JavaScript code with mocha in the CLI. All my instructions are for OS X, but setting it up should be very similar on Linux and (maybe) on Windows as well.

First of all, you need node.js to run JavaScript code in the terminal. You can download the source code and compile it yourself, but I'd recommend using Homebrew and let it do the job for you.

$: brew install node

At the time of this writing my current node version is 0.6.6. You can check your node.js version by running this command:

$: node -v
v0.6.6

Next you need node's package management tool (npm). Your version of node may include npm, I list this step here in case it does not. Installing it is super easy, just follow the instructions on their web site and in your terminal.

With these two steps you're ready to roll. Create the project directory, cd into it and start moving in. Create a "src" and a "test" directory. You need to install mocha and should.js as npm packages. Having sinon.js - an excellent spying framework - wouldn't hurt either. Create your spec and source file and you are ready to test drive your app with mocha.

I really wanted to help you - Dear Reader - so I created this shell script to make your life easier. Create a directory, cd into it and run the command below in your terminal:

   curl -L http://git.io/setup_mocha_project | sh

If everything goes OK, you will see this:

create the src directory...
create the test directory...
write the package.json file...
install npm packages...

create a sample spec file...
create a sample src file...
run the spec with mocha...
  .

  ✔ 1 tests complete (1ms)

run the spec with list reporter...

   Person should be able to say hello: 1ms

  ✔ 1 tests complete (2ms)

Let's add one more feature to our Person object. Open up the test/person_spec.js file - it was created by the shell script above - and add the "can say good night" spec:

var should = require('should');
var Person = require(__dirname + '/../src/person');

describe('Person', function() {
  it('should be able to say hello', function() {
    var Person = global.theApp.Person();
    var personInstance = new Person();
    var message = personInstance.sayHelloTo('adomokos');

    message.should.equal('Hello, adomokos!');
  });

  // Add this spec
  it('can say good night', function() {
    var Person = global.theApp.Person();
    var personInstance = new Person();
    var message = personInstance.sayGoodNight();

    message.should.equal('Good night!');
  });
});

Run the mocha specs with this command:

$: ./node_modules/mocha/bin/mocha

The error is obvious: the Person object does not yet have the method "sayGoodNight".

  ..

  ✖ 1 of 2 tests failed:

  1) Person can say good night:
     TypeError: Object [object Object] has no method 'sayGoodNight'

Let's fix it by adding the missing method to the Person object:

global.theApp = {};

global.theApp.Person = function() {

  var Person = function() {
   this.sayHelloTo = function(anotherPerson) {
      return 'Hello, ' + anotherPerson + '!';
    };

   // Add this method
   this.sayGoodNight = function() {
     return 'Good night!';
   };
  };

  return Person;

};

When I run the specs again, they all pass.

  ..

  ✔ 2 tests complete (2ms)

You can try other reporters as well. The "list" reporter will give you the documentation text:

$: ./node_modules/mocha/bin/mocha -R list

Try the landing reporter, I found its output unexpected but really cool!

$: ./node_modules/mocha/bin/mocha -R landing

The steps once more:

  1. Make sure you have node.js installed
  2. Check for npm as well
  3. Create your project directory and cd into it
  4. Run this script $: curl -L http://git.io/setup_mocha_project | sh
  5. Execute the specs with $: node_modules/mocha/bin/mocha
And I almost forgot: mocha will pick up CoffeeScript files as well.

Enjoy!

 

::: Update (01/24/2012):
I asked TJ Holowaychuck, the author of Mocha of what thoughts he had on my blog post. He recommended adding a "test" script to the package.json file making it easier to run the specs. I made that change: npm test executed in the terminal should run all your specs under the test directory.

Monday, January 2, 2012

The Tic-Tac-Toe Game

I've been pretty busy lately working on this Tic-Tac-Toe game. It all started as a project to learn CoffeeScript, Backbone.js and turned into a big journey into JavaScript and node.js.

I am test-driving the code with jasmine-node, a great node adapter to Jasmine BDD.
The computer's moves are quite predictable, I will work on that in the future.


Won: 0
Lost: 0
Tie: 0

Enjoy!

Wednesday, December 14, 2011

(More) Specific Stubbing with RSpec

A couple of months ago we had to write code for the following feature: a company would like to reward its most valuable customers by giving them credit which they can use in their future orders.

We came up with the following solution:
class GivesCreditToPreferredCustomers
  def self.for_large_orders(sales_amount, added_credit)
    preferred_customers = Customer.has_large_purchases(sales_amount)
    preferred_customers.each do |customer|
      customer.add_credit added_credit
    end
  end
end

class Customer
  attr_reader :total_credit

  def self.has_large_purchases(sales_amount)
    puts "AR query to find buyers with large purchases"
  end

  def add_credit(amount)
    @total_credit = 0 if @total_credit.nil?
    @total_credit += amount
  end
end

describe GivesCreditToPreferredCustomers do
  specify "for large orders" do
    sales_amount = 10000
    credit_given = 100
    found_customer = Customer.new
    Customer.stub(:has_large_purchases) \
            .and_return [found_customer]

    GivesCreditToPreferredCustomers \
            .for_large_orders(sales_amount, credit_given)

    found_customer.total_credit.should == credit_given
  end
end

Take a look at the lines where the Customer's :has_large_purchases method is being stubbed: "Customer.stub(:has_large_purchases).and_return([found_customer])".
Everything is passing there, even though I have not specified any arguments. Of course: when you don't specify arguments, RSpec will take any arguments (or no arguments) and return the canned response.

A couple of months passes by and a new requirement comes in: we need to look at only the last 3 months of purchases, otherwise the company is giving away too much credit to its customers. The look back period is the same to all customers, it's safe to put it in the GivesCreditToPreferredCustomers class.

You would obviously start with modifying the spec, but your co-worker wants to get this done really quick and updates the application code like this:

class GivesCreditToPreferredCustomers
  LOOK_BACK_PERIOD = 3
  def self.for_large_orders(sales_amount, added_credit)

    # the has_large_purchases scope now takes two arguments
    preferred_customers = Customer.has_large_purchases(sales_amount, LOOK_BACK_PERIOD)
    
    preferred_customers.each do |customer|
      customer.add_credit added_credit
    end
  end
end

I execute the spec and everything passes:
.

Finished in 0.00063 seconds
1 example, 0 failures

Wow! That's quite a bit of change and nothing failed. Yet.

Let's make sure that only those messages are stubbed that have the correct arguments. I add the with() method to the stub's method chain:

describe GivesCreditToPreferredCustomers do
  specify "for large orders" do
    sales_amount = 10000
    credit_given = 100
    look_back_period = 3
    found_customer = Customer.new

    Customer.stub(:has_large_purchases) \
            # stub with arguments
            .with(sales_amount, look_back_period) \
            .and_return [found_customer]

    GivesCreditToPreferredCustomers \
            .for_large_orders(sales_amount, credit_given)

    found_customer.total_credit.should == credit_given
  end
end

Everything passes in the spec but we are now stubbing messages only where the :has_large_purchases method is called with the passed in sales amount (10,000) and the correct look back period (3).
.

Finished in 0.00062 seconds
1 example, 0 failures

Let's see what happens when the LOOK_BACK_PERIOD is changed to 2 due to a new requirement from the customer:

F

Failures:

  1) GivesCreditToPreferredCustomers for large orders
     Failure/Error: preferred_customers = Customer.has_large_purchases(sales_amount, LOOK_BACK_PERIOD)
       received :has_large_purchases with unexpected arguments
         expected: (10000, 3)
         got: (10000, 2)
     # ./describe_stub_spec.rb:5:in `for_large_orders'
     # ./describe_stub_spec.rb:38:in `block (2 levels) in '

Finished in 0.00104 seconds
1 example, 1 failure

This would happily pass with a stub where I don't specify the arguments but it fails here where the stub argument is strictly defined.

Adding the argument is a little bit more work but the benefits are huge: you are exercising not only the message sent to the object but the arguments that the message is sent with.

Happy stubbing!

You can review the example I created for this blog post in this Gist.

Tuesday, October 11, 2011

Running Fast RSpec Tests With and Without Rails

So you got out of the controller and from Active Record and you're ready to test your services without Rails?

I'll describe how you can trust your fast Rails specs by defining classes safely and ways you can execute them with or without Rails. All of my examples are a continuation of my previous blog post, I recommend reading that first before you proceed with this one.

The FindsUsers service is very simple:
# lib/service/finds_users.rb
module Service
  class FindsUsers
    def self.all
      User.active.map { |user| ::DTO::User.new(user) }
    end
  end
end
And this is how I created the first spec without Rails:
# spec/units/service/finds_users_spec.rb
APP_ROOT = File.expand_path(File.join(File.dirname(__FILE__), "..", "..", ".."))
$: << File.join(APP_ROOT, "lib")
$: << File.join(APP_ROOT, "spec/units")

module ActiveRecord
  class Base; end
end
class User < ActiveRecord::Base; end

require 'ostruct'
require 'service/finds_users'
require 'factory/for_user'
require 'dto/user'

describe Service::FindsUsers do
  let(:users) { Factory::ForUser.make_two }

  describe "converts the found users to DTO::User" do
    before { User.stub(:active).and_return users }
    subject { Service::FindsUsers.all }

    its(:size) { should == 2 }
    its(:first) { should be_instance_of ::DTO::User }
    its(:last) { should be_instance_of ::DTO::User }
  end
end
Please take a look at line 9, where I declared the User class. I need to do this since I don't reference the application's Active Record models in these specs. I don't need to, all I care is that it's some kind of User class that has an :active class method on it.

I also declared a test dummy for ActiveRecord::Base. It doesn't matter what it does, I just want to make sure my User class declaration is as close to the original Active Record model as possible.

When I run the specs they all pass:
...
Finished in 0.00223 seconds
3 examples, 0 failures
rspec spec/units/service/finds_users_spec.rb 0.29s user 0.09s system 96% cpu 0.392 total

It works great, but there are a few lines that will be used in other specs. I move those into the spec/units/spec_helper.rb file.
# spec/units/spec_helper.rb
APP_ROOT = File.expand_path(File.join(File.dirname(__FILE__), "..", ".."))
$: << File.join(APP_ROOT, "lib")
$: << File.join(APP_ROOT, "spec/units")
$: << File.join(APP_ROOT, "spec/units/factory")

require 'ostruct'

# Defining an ActiveRecord::Base dummy for models
module ActiveRecord
  class Base; end
end
Now my finds_users_spec.rb file is shorter and cleaner:
# spec/units/service/finds_users_spec.rb
require 'units/spec_helper'

# ActiveRecord::Base is defined in spec/units/spec_helper.rb
class User < ActiveRecord::Base; end

require 'service/finds_users'
require 'factory/for_user'
require 'dto/user'

describe Service::FindsUsers do
  let(:users) { Factory::ForUser.make_two }

  describe "converts the found users to DTO::User" do
    before { User.stub(:active).and_return users }
    subject { Service::FindsUsers.all }

    its(:size) { should == 2 }
    its(:first) { should be_instance_of ::DTO::User }
    its(:last) { should be_instance_of ::DTO::User }
  end
end
Testing the FindsDiscussion service is just as simple:
# spec/units/service/finds_discussion_spec.rb
require 'units/spec_helper'

# ActiveRecord::Base is defined in spec/units/spec_helper.rb
class Discussion < ActiveRecord::Base; end

require 'service/finds_discussion'
require 'factory/for_discussion'
require 'dto/discussion'
require 'dto/comment'

describe Service::FindsDiscussion do
  let(:discussion) { Factory::ForDiscussion.make_one }

  describe "looks up a discussion and converts it to DTO" do
    before { Discussion.stub(:find).and_return discussion }
    subject { Service::FindsDiscussion.for 24 }

    it { should be_instance_of ::DTO::Discussion }
  end
end
I also need to declare the Discussion class here, so I can stub it out for my service.

They all pass when I execute the entire spec/units suite:

....
Finished in 0.00513 seconds
4 examples, 0 failures
rspec spec/units 0.28s user 0.09s system 96% cpu 0.387 total

BUT WAIT!!

My User Active Record model has the scope :active that I verify it by loading up Rails in this spec:
# spec/models/user_spec.rb

# This spec is using the spec/spec_helper.rb file that loads up Rails with Active Record!
require 'spec_helper'

describe User do
  it { should respond_to :active }
end
I run its slow AR spec and a unit spec with this command in the terminal:
$: time rspec spec/models/user_spec.rb spec/units/service/finds_users_spec.rb
It takes a little while - 4 seconds - but everything passes.

....
Finished in 0.04386 seconds
4 examples, 0 failures
rspec spec/models/user_spec.rb spec/units/service/finds_users_spec.rb 3.49s user 0.59s system 100% cpu 4.083 total

But when I change the files around - executing the spec that does not need Rails first and the model spec that uses Rails second:
$: time rspec spec/units/service/finds_users_spec.rb spec/models/user_spec.rb
The specs are executed fast, but the AR model spec failed:

...F
Failures:

  1) User
    Failure/Error: it { should respond_to :active }
      expected #<User:0x00000100a53538> to respond to :active

    # ./spec/models/user_spec.rb:5:in `block (2 levels) in <top (required)>'

Finished in 0.00248 seconds
4 examples, 1 failure

You might be puzzled why this spec failed, but the explanation is rather simple: in the first case we ran the AR spec first. It loaded up and used the AR User model, the spec passed. Then we opened the User class in our fast spec, stubbed out a method on the User Active Record model and the service spec passed as well.

In the second case we defined our User class for our fast spec, executed the spec and they all passed. Then the AR model spec picked up the already declared User class - which was not the AR User model - and since it did not have the :active scope defined, it failed.

This is exactly what happened when we started executing all our specs - both non-Rails and Rails specs together - on our build server. The spec execution order was different on CentOS and different on our local OS X development environment. Everything passed locally, but had quite a few errors on the build server. We obviously had to find a solution.

First of all, redefining classes all over the specs just wasn't a good idea. I moved all my redefined classes into spec/units/spec_helper.rb from the different specs.
# spec/units/spec_helper.rb
APP_ROOT = File.expand_path(File.join(File.dirname(__FILE__), "..", ".."))
$: << File.join(APP_ROOT, "lib")
$: << File.join(APP_ROOT, "spec/units")
$: << File.join(APP_ROOT, "spec/units/factory")

require 'ostruct'

# Defining an ActiveRecord::Base dummy for models
module ActiveRecord
  class Base; end
end

# I moved the redefined classes here
class User < ActiveRecord::Base; end
class Discussion < ActiveRecord::Base; end
Look at line 15 and 16 in the spec_helper, the redefined classes are now in one single place as opposed to having them scattered all over the specs.
I ran the fast specs without Rails again and they were all passing.

All I had to do to get the specs passing regardless of file order was including the Rails-aware spec_helper into the spec/units/spec_helper.rb file that loaded up Rails with the real Active Record models (line 2 below):
# Including the full stack spec_helper, loads the AR models with Rails
require 'spec_helper'

# spec/units/spec_helper.rb
APP_ROOT = File.expand_path(File.join(File.dirname(__FILE__), "..", ".."))
$: << File.join(APP_ROOT, "lib")
$: << File.join(APP_ROOT, "spec/units")
$: << File.join(APP_ROOT, "spec/units/factory")

require 'ostruct'

# Defining an ActiveRecord::Base dummy for models
module ActiveRecord
  class Base; end
end

# I moved the redefined classes here
class User < ActiveRecord::Base; end
class Discussion < ActiveRecord::Base; end
Now when I execute the specs starting with the fast spec first it loads up Rails and in about 4 seconds I know that all the specs are passing regardless of what file order was used at execution time.

This change alters the User and Discussion class declarations as well. They are not redefined classes any more, they are open classes in the execution context. I am not modifying their behavior, I am just opening up the classes and leaving them unchanged.

A script in the build process can change the spec/units/spec_helper.rb file to include the full stack spec_helper.rb file.

This might seem like a lot of voodoo for some, but I am working on a fairly large Rails app and it takes about 23 seconds to execute one spec with Rails. I believe with just a little bit of meta programming trick you can enjoy very fast feedback loop making you more effective at writing software.

Wednesday, September 7, 2011

Get out of my Controller! And from Active Record, too!

I wrote about Running Rails Rspec Tests Without Rails a couple of months ago. The examples I used were very high level and focused on stubbing out Rails in my tests in order to achieve rapid feedback.

A couple of months have passed and the topic is getting more and more buzz thanks to Corey Haines and Robert "Uncle Bob" Martin.

I've been getting many questions on how I abstract away from Rails' Active Record, how do I use service objects to lighten up my controllers. I'll try to describe all that in this blog post.

Imagine an application where you have topics that people can comment on. The Active Record models are something like this:
class User < ActiveRecord::Base
  # These fields are defined dynamically by ActiveRecord
  attr_accessor :id, :full_name
end

class Discussion < ActiveRecord::Base
  # These fields are defined dynamically by ActiveRecord
  attr_accessor :id, :title, :body, :comments
end

class Comment < ActiveRecord::Base
  # These fields are defined dynamically by ActiveRecord
  attr_accessor :id, :text, :entered_by
end
And here is their relationships:

This is pretty simple: Discussion has many comments and a comment was entered by a user. Fantastic!
But what do you do when your customer comes to you and asks you to get not only the comments for a given discussion but she would like to see each user with their comments made on the specific discussion.

Here is the page layout:



The Active Record models will perfectly match the view hierarchy on the left. But you are looking at the same data from a different angle on the right hand side.
How are you going to get that data into the view?

Here are some of your options:
  1. Create a view helper that grabs the user's comments from the DB
  2. Add a new field to the User AR model to hold the comments
  3. Use Plain Old Ruby Object (PORO) models on top of AR models and use service objects
Number one is beyond bad. You are actually iterating through the users and hitting the database for every single user to get their comments. BAD! Never do that! It's a very expensive operation: connection is opened, query is executed, AR models are built up from the result set. You already have the data in memory. Use it!

Number two is better but I don't like that either. By adding a field to the User AR model you can do all the data processing in the controller and present that data to the view. This way the view iterates over the users and for each user it iterates over its comments. There is no lookup from the view but you are polluting the AR model with a field that is specific to one particular view. The User AR model is a core object in your application, you want to keep it very clean. Other developers should not be puzzled by an attr_accessor called :comments.

Here is what I'd do: create small model objects that wrap the AR models. Use service objects to populate these POROs and prepare them exactly as the view needs it. Then the view is very simple: it iterates over these model objects and uses their properties.
I call these PORO objects DTOs or Data Transfer Objects. They serve custom data from the model to the view.

Here is how a UserDTO looks:
module DTO
  class User
    attr_reader :source_object, :id, :full_name
    attr_accessor :comments
    def initialize(source_object)
      @source_object = source_object
      @id = source_object.id
      @full_name = source_object.full_name
    end
  end
end
I keep a reference to the original AR model through the @source_object variable. Whatever field I can populate from the source object I do that in the object's initializer. But in our case there is an extra field that does not exist in the source model: comments. This field is declared but not yet populated. The service object will take care of that.

The controller's index action has to do three things:
  • Get the currently viewed discussion from the database
  • Retrieve all the users
  • Find the users' comments under the current discussion
You could place all the code into the controller's action, but you'll have a bloated controller thats very hard to test and the logic will be impossible to reuse.
I use very granular service objects from the controller.
# Services used in the app
module Service
  class FindsDiscussion
    def self.for(id)
      # This is very high level
      ::DTO::Discussion.new(Discussion.find(id))
    end
  end

  class FindsUsers
    def self.all
      User.all.map { |user| ::DTO::User.new(user) }
    end
  end

  class SetsComments
    def self.on_users(users, comments)
      # There is no trip to the DB!
      users.each do |user|
        user.comments = comments.select do |comment|
          user.source_object.id == comment.source_object.entered_by
        end
      end
    end
  end
end
Look at how small they are! The first and second service looks up data in the database, but the third one is using an in memory lookup. This is how I am saving the trip to the data store.
SRP is strictly followed, these little logic classes are super easy to test and using them from the controller is straightforward:
class DiscussionsController < ApplicationController
  attr_reader :users, :discussion

  def index
    @users = Service::FindsUsers.all
    @discussion = Service::FindsDiscussion.for(params[:id])
    Service::SetsComments.on_users(@users, @discussion.comments)
  end
end
You are creating many more small classes, but that's OK. They are easy to understand, easy to test and you can use them like little LEGO blocks to construct the logic your controller needs.

You can find the examples I used in the blog post in this Gist.