Thanks Scott. I refactored it today to use what I called a
QueryAdapter, namespaced inside the model. It basically subclasses
Hash, takes the params from the controller into the constructor, and
becomes the hash to be sent to find_all.
I feels much better, as I now have the code that's coupled to the
database in one place, but I'd welcome feedback:
# VenuesController
def get_venues
Venue.paginate( :all, Venue::QueryAdapter.new(params) )
end
# Responsible for mapping a hash of parameters that will
typically be POSTed to a controller into a hash that can be sent to
find(:all)
# containing SQL clauses in :conditions / :order.
# This helps us decouple the view / controller layers from any
database specific stuff.
class Venue::QueryAdapter < Hash
def initialize(params)
parse params
self.merge!(get_find_params)
end
private
def parse(params)
@sort_column = params[:sort]
@city_id = params[:city_id]
@name = params[:name]
@page = params[:page]
end
def get_find_params
find_params = {}
find_params.merge!( :order => get_order_clause ) if
get_order_clause.length > 0
find_params.merge!( :conditions => get_where_clause ) if
get_where_clause.length > 0
find_params.merge!( :page => @page )
return find_params
end
def get_where_clause
clause = []
clause << "city_id = [EMAIL PROTECTED]" if @city_id
clause << "name like '[EMAIL PROTECTED]'" if @name
return clause.join(" AND ")
end
def get_order_clause
clause = []
clause << 'created_on DESC' if @sort_column == 'created_at'
clause << 'name ASC'
return clause.join(", ")
end
end
cheers,
Matt
----
http://blog.mattwynne.net
http://songkick.com
In case you wondered: The opinions expressed in this email are my own
and do not necessarily reflect the views of any former, current or
future employers of mine.
On 16 Aug 2008, at 00:19, Scott Taylor wrote:
On Aug 15, 2008, at 9:29 AM, David Chelimsky wrote:
On Aug 15, 2008, at 6:46 AM, Matt Wynne <[EMAIL PROTECTED]> wrote:
On 15 Aug 2008, at 12:25, David Chelimsky wrote:
Hey Matt - welcome!
The paginate() method lives on the model class, so there's nothing
stopping you from wrapping those calls in methods on the model,
slinging around the params object.
# CityController
def get_cities
City.paginate_all(params)
end
# City
def self.paginate_all(params)
self.paginate(:all, get_find_params(params).merge!(:page =>
params[:page]))
end
etc
Aha. Cool, thanks.
For my next question: how do I go about driving out change to the
model, spec-first?
I'm thinking I would call (in my spec)
City.should_receive(:paginate).with(:conditions => "name like '%
#{test_params[:name}%'" .... )
City.paginate_all(test_params)
Thereby covering the code in get_find_params()
Is that the right approach?
That's probably how I would do it. Might also consider wrapping
the params in a separate object that manages the extraction.
That's how I've started doing it - putting sql statements in a module:
http://gist.github.com/5675
This allows me to test the sql statements seperately from the
actual finder.
Also - just to give you the heads up - You should almost never use
literal string substitutions in sql statements - it allows for sql
injection attacks:
http://en.wikipedia.org/wiki/Sql_injection
Best,
Scott Taylor
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users
