Topic: Refactoring Searches

OK, I switched to will_paginate from the default pagination, and I moved the search to the model like RyanB did in his Railscast. I'm using these searches in four similar but different ways, and I'm looking for a nice Ruby way to DRY this up:

  # in my model

  DEFAULT_ITEMS_PER_PAGE = 12

  def self.recent_by_location(location, page, per_page = DEFAULT_ITEMS_PER_PAGE)
     paginate(:conditions => ['dotted_locations LIKE ?', location.dotted_ids + '%'],
              :order => 'created_at DESC', :page => page, :per_page => per_page)
  end

  def self.recent_by_taxon(taxon, page, per_page = DEFAULT_ITEMS_PER_PAGE)
     paginate(:conditions => ['dotted_taxa LIKE ?', taxon.dotted_ids + '%'],
              :order => 'created_at DESC', :page => page, :per_page => per_page)
  end
 
  def self.recent_by_user(user, page, per_page = DEFAULT_ITEMS_PER_PAGE)
     paginate(:conditions => ['user_id=?', user.id ],
              :order => 'created_at DESC', :page => page, :per_page => per_page)
  end

  def self.recent(page, per_page = DEFAULT_ITEMS_PER_PAGE)
     paginate(:order => 'created_at DESC', :page => page, :per_page => per_page)
  end


Suggestions are appreciated. My first thought was to put it all in one method and use if or case to handle the variations, but maybe there's a better way.

(BTW, the dotted_id stuff you see there is a nice way to work with trees. I've used it before in other languages, but I found a good Ruby on Rails writeup for it here.)

Re: Refactoring Searches

How about accepting conditions in the "self.recent" call and then you just need to call that through the other methods:

DEFAULT_ITEMS_PER_PAGE = 12

def self.recent_by_location(location, page, per_page = DEFAULT_ITEMS_PER_PAGE)
  recent(page, per_page, ['dotted_locations LIKE ?', location.dotted_ids + '%'])
end

def self.recent_by_taxon(taxon, page, per_page = DEFAULT_ITEMS_PER_PAGE)
  recent(page, per_page, ['dotted_taxa LIKE ?', taxon.dotted_ids + '%'])
end

def self.recent_by_user(user, page, per_page = DEFAULT_ITEMS_PER_PAGE)
  recent(page, per_page, ['user_id=?', user.id ])
end

def self.recent(page, per_page = DEFAULT_ITEMS_PER_PAGE, conditions = nil)
  paginate(:order => 'created_at DESC', :page => page, :per_page => per_page, :conditions => conditions)
end

Railscasts - Free Ruby on Rails Screencasts

Re: Refactoring Searches

Thanks Ryan. Do you know of a good way to unit test this sort of method? I guess you'd need a fixture with several entries, and verify that a certain number of them come back and in a certain order?

Re: Refactoring Searches

Testing these kinds of methods can be tricky. Fixtures is one solution, but I hate to have tests rely heavily on the content of the fixtures. Another solution is mocking, but the parameters may vary slightly resulting in brittle tests.

One other solution is what I usually do and that is to create the necessary models in the test:

def test_recent
  Item.delete_all
  item1 = Item.create(:created_at => 3.months.ago)
  item2 = Item.create(:created_at => 2.months.ago)
  assert_equal [item2, item1], Item.recent
  assert_equal [item2], Item.recent(1, 1)
  assert_equal [item1], Item.recent(2, 1)
end

Not the cleanest but hopefully you get the idea.

Railscasts - Free Ruby on Rails Screencasts

Re: Refactoring Searches

Thanks Ryan, I'll give that test a try.