graeme nelson

Recently, I’ve vowed to remove most of the model.find methods from my controller. I am not talking about simple find statements like:

 model.find(1) 
 model.find_by_title("my title") 

I am talking about find methods like so:

  model.find(:all, :conditions => ["status = ?", my_status])
  model.find(:all, :conditions => ["title = ?", my_title])

I would add methods on the model called, find_all_by_status(status) and find_all_by_title(title). Now, you may be thinking that these are simple find statements too. And I would agree with you. But in my opinion it makes the controller code look dirty. What would you rather see in your controller?

  model.find(:all, :conditions => ["status = ?", my_status])

~OR~

  model.find_all_by_status(my_status)

Which one can you comprehend faster? For me, the first statement takes some time for my brain to process, where as the second statement doesn’t not. Ok, if this isn’t enough to convince you. What if you introduce a new boolean attribute on your model called ‘active’ and the logic is to only show active model instances? Now you have to change all the places where you have:

  model.find(:all, :conditions => ["status = ?", my_status])

 ~TO~

  model.find(:all, :conditions => ["status = ? and active = ?", my_status, true])

You say you were thinking ahead and have a private method on your controller to handle this call, so you only have to make the change in one place. I would say congratulations, but I would point out that I think the controller knows too much about the model and thus breaks encapsulation.

I would like to hear what other think, and how they handle this situation.

I’ve been using rspec for a few months now, and I really like it. I recommend trying it out if you haven’t.

I figure it might be nice to share how I organize controller specs, and get some feedback on how others might organize their specs. I like having one test per specification like so:

specify "should redirect to new session url" do
   response.should_redirect_to new_session_url
end

I also like calling the controller action in the setup method.

setup do
   ... any necessary setup info
   post :update, :id => model, :model => params
end

Here’s what a context might look like in all its glory.

context "Create with a valid product and authenticated user" do
  include ProductsControllerSpecHelper
  controller_name :products

  setup do
    @product = mock(:product, :null_object => true)
    Product.stub!(:new).and_return(@product)
    @product.should_receive(:save!).and_return(true)
    @product.should_receive(:categories).and_return([])
    authenticate_user_mock

    post :create, :product => valid_product_attributes

  end

  specify "should redirect to edit" do
    response.should_redirect_to edit_product_url(@product)
  end

  specify "should assign product" do
    assigns[:product].should_not_be_nil
  end

end