Writing Better Rails

by Craig Buchek --- August 8, 2011 --- St. Louis Ruby Users Group

• Writing better cucumber stories
• Writing better cucumber step definitions
• Writing better specs
• Writing better controllers
• Ideas on overweight models
• Writing better views

• You're cuking it wrong:

  Scenario: Reset my password
    Given a user with nickname "john" and email "john@john.com"
    And I am on the reset password page
    When I fill in "Nickname" with "john"
    And I press "Send me reset password instructions"
    And "john@john.com" opens the email with subject "Reset password instructions"
    And I follow "sign in to glu" in the email
    And I fill in the following:
      | Password              | pass1235      |
      | Password confirmation | pass1235      |
    And I press "Change my password"
    Then I should see "Your password was changed successfully."
    And I should be on my dashboard page

• You're cuking it right:

  Scenario: suggestion from logged in user via "Stay Tuned" modal
    Given I am logged in
    When I submit feedback via the stay tuned dialog
    Then an email gets sent to the feedback email address
    And the email contains my username
    And the email contains my comment

• Avoid using steps in step definitions:

FEEDBACK_COMMENT = 'This is the text of my comment'
When /^I submit feedback via the stay tuned dialog$/ do
  And %(I go to the question of the day page)
  And %(I follow 'submit your own question')
  And %(I should see a message saying stay tuned)
  And %(I fill in 'Comment' with "#{FEEDBACK_COMMENT}" within '.ui-dialog')
  And %(I press 'Submit' within '.ui-dialog')
end

• Use capybara methods directly:

FEEDBACK_COMMENT = 'This is the text of my comment'
When /^I submit feedback via the stay tuned dialog$/ do
  visit question_of_the_day_path
  click_link 'submit your own question'
  page.should have_content('stay tuned')
  within ".ui-dialog" do
    fill_in 'Comment', :with => FEEDBACK_COMMENT
    click_button 'Submit'
  end
end

• Learn Capybara methods!
  • Actions
  • Scoping - within variants
  • Matchers - have_selector variants and options
  • Finders and node manipulation

• Use let instead of instance variables
  • Note that let is lazy
  • Use let! if you need eager evaluation
• Use subject
• Use contexts
  • Separate setup and tests for different situations
• Use stubs/mocks as appropriate
  • Never for the subject class
  • Not for immediate collaborator
  • Always for controller

  describe '#read_and_unread_messages' do
    subject { current_user.mailbox }
    let(:current_user) { Factory :enrolled_user }
    let(:other_user) { Factory :enrolled_user }
    let(:message1) { Factory.build :message, :to => current_user, :from => other_user, :body => "message 1" }
    let(:message2) { Factory.build :message, :to => other_user, :from => current_user, :body => "message 2" }
    it 'includes messages sent to me' do
      subject.read_and_unread_messages.to_a.should include(message1)
    end
    it 'does not include messages I sent' do
      subject.read_and_unread_messages.to_a.should_not include(message2)
    end
  end

describe Mailbox do
  describe "#delete_message" do
    context 'when the message is in the deleted folder' do
      it 'saves the mailbox'
      it 'removes the message id from the deleted folder'
      it 'tells the message to decrement its reference count'
      it 'decrements the deleted messages count'
    end
  end
end

describe Mailbox::Folder::MessagesController do
  describe "#index" do
    let(:messages) { mock 'messages array' }
    let(:params) { {:folder_id => 'deleted'} }
    before do
      subject.stub(:messages) { messages }
    end
    it 'responds with the messages' do
      subject.should_receive(:respond_with).with(messages)
      get :index, params
    end
  end
end

• Use Decent Exposure
  • Simplifies getting the resources you are working with
  • Defines an API between the controller and views
  • A bit too much "magic" at times
  • Note that exposed resources are lazily-evaluated methods
• Use rescue_from DocumentNotFound, :with => :render_404
  • In your ApplicationController
• Use respond_with
  • Use in conjunction with respond_to
  • Does the Right Thing for HTML, JSON, XML, etc.
    • Replaces render for HTML
  • Can handle redirects
  • Can handle per-type exceptional cases
  • Can be a bit tricky with nested resources

class DeletedMessagesController < ApplicationController
  respond_to :html, :only => [:index, :destroy]
  respond_to :json
 
  expose(:messages) { current_user.mailbox.deleted_messages }
  expose(:message)
 
  def index
    respond_with messages
  end
 
  def destroy
    current_user.mailbox.delete_message(message)
    respond_with message, :location => deleted_messages_path
  end
end

• Rails community and best practices encourages skinny controllers
  • Keep business logic out of controllers
  • Controllers should only set up models, do one thing with them, and send output to views
• Skinny controllers means moving stuff to models
• But sometimes models get TOO fat

• Model class gets disorganized
  • Hard to find what you need in the file
• Model class does too much
  • Probably violating the Single Responsibility Principle (SRP)
  • Behavior and persistence are 2 separate concerns

• Instead of starting with the model, start with the controller
  • Only write a model method/accessor/field if a controller (or other model) needs it
• DataMapper encourages this somewhat
  • You specify the fields you need explicitly
• ActiveRecord encourages the opposite
  • Because you have to create a migration to get ANY data in your schema

• Ended up being difficult to find where the functionality exists within files
• We put the modules in lib
  • Probably should have stayed in app/models

• Split User versus Profile (with no delegation)
• Confusion:
  • Schizophrenia -- which piece did we put where?
  • Which IDs do we pass to API clients?
• Both classes are still too large

• Split DAO from business logic.
• Problem: which piece does validations?
  • I think the business logic should probably do it
    • Include ActiveModel::Validation
    • But then business logic has intimate knowledge of the persistence model

• Composite object with delegation
  • Similar to modules
  • Can lead to self schizophrenia
• Delegation of methods to the contained objects

class User
  field :mailbox
  delegate :messages, :to => :mailbox

• DCI - Data, Context, Interaction
• Only pull in the module that we need for the current operation
• Almost more like prototype-based OOP than class-based OOP
• All behavior comes from a role -- the model only contains the DAO
• Example - User class contains the DAO.
  • Buyer - adds logic to a user making a purchase.
    • Controller then looks like this:
      • current_user.extend Buyer; current_user.add_to_cart(product_id, quantity)
  • NewsletterManager - adds logic to a user managing a newsletter
• http://andrzejonsoftware.blogspot.com/2011/02/dci-and-rails.html
  • Suggests that everything be based off of just a couple objects:
    • current_user
    • website or application
• Problem: doesn't work when the roles require other data

• Command pattern is ideal for encapsulating complex actions

• ActiveService

• We are using HAML to make HTML look clean
• We just started using SASS to simplify CSS
  • Variables for colors
  • Mixins
  • Nesting
• Judicious use of partials
• Use helpers to limit conditionals within views
• Some improvement using Decent Exposure instead of @instance_variables
  • Helps us to write better helpers/partials
• Future possibilities:
  • Declarative forms
    • Formtastic or SimpleForm
  • Declarative views
  • Declarative lists
  • Declarative JSON/XML views

• Controllers mix operations on collections and single items
  • The index action lists all items; everything else acts on a single item
    • POST to the collection to create a new item
  • The index and create actions work on a different URL than the others
    • Makes it confusing to remember which of the 2 *_path/*_url helpers to use
• We should allow DELETE on the collection to delete all the items in the collection
• Mapping between REST and actions doesn't make as much sense as it could
  • I'm starting to like Sinatra's syntax better for this
    • Explicitly list GET, POST, PUT, DELETE method
• We have a client view associated with every action / REST verb, except for destroy / DELETE
  • I propose a delete action that merely displays a form confirming the deletion
    • Would be good for non-JavaScript clients (people with disabilities, Cucumber environments without JavaScript)
• Why does the resource in every controller have a different name?
  • Could we just expose(:resources) and/or expose(:resource) in every controller?
  • It would be easier to share views if all the controllers used the same name for the resource we're working with

• http://solnic.eu/2011/08/01/making-activerecord-models-thin.html
• http://jamesgolick.com/2010/3/14/crazy-heretical-and-awesome-the-way-i-write-rails-apps.html
  • Uses dependency injection and service objects, which I find mostly unnecessary
• http://blog.astrumfutura.com/2008/12/the-m-in-mvc-why-models-are-misunderstood-and-unappreciated/
• http://andrzejonsoftware.blogspot.com/2011/02/dci-and-rails.html

• You're Cuking It Wrong
• You're almost cuking it...
• 15 Expert Tips for Using Cucumber

• Pure RSpec Presentation by L4rk

• The RSpec Book
• Crafting Rails Applications
• The Rails 3 Way
• The Cucumber Book