def merge_query!(options)
return unless params[:query]
statements = []
values = []
conditions = options[:conditions] || [""]
@properties.each do |property|
next unless property[:type] == :string
statements << "#{property[:name]} LIKE ?"
values << "%#{params[:query]}%"
end
conditions[0] += " AND " unless conditions == [""]
conditions[0] += statements.join(" OR ")
conditions += values
options.merge!(:conditions => conditions) unless conditions == [""]
end
def merge_filter!(options)
return unless params[:filter]
statements = []
values = []
conditions = options[:conditions] || [""]
params[:filter].each_pair do |key, value|
@properties.each do |property|
next unless property[:name] == key.to_sym
next unless property[:type] == :boolean
statements << "#{key} = ?"
values << (value == "true")
end
end
conditions[0] += " AND " unless conditions == [""]
conditions[0] += statements.join(" AND ")
conditions += values
options.merge!(:conditions => conditions) unless conditions == [""]
end
Refactorings
No refactoring yet !
Colin Curtin
November 5, 2009, November 05, 2009 05:22, permalink
No rating.
Login to rate!
Here's a first draft. I don't know if it works, but it looks right.
class Conditions
def initialize(parameters)
@parameters = [parameters].flatten.compact.reject{|p| p.empty?}
@values = []
end
def add(param, val)
@parameters << param
@values << val
end
# Assumes first param is going to be "and"ed to the rest.
def to_cond(join="AND")
first = @parameters.shift
last = "(%s)" % @parameters.join(") #{join} (")
stmt = [first, last].join(" AND ")
[stmt] + @values
end
def to_hash(join)
{:conditions => to_cond(join)}
end
def empty?
@parameters.empty?
end
end
def merge_query!(options)
return unless params[:query]
cond = Conditions.new(options[:conditions])
@properties.select{|p| p[:type] == :string}.each do |p|
cond.add("#{p[:name]} LIKE ?", "%#{p[:query]}%")
end
options.merge!(cond.to_hash("OR")) unless cond.empty?
end
def merge_filter!(options)
return unless params[:filter]
cond = Conditions.new(options[:conditions])
params[:filter].each_pair do |key, value|
@properties.select do |p|
p[:type] == :boolean && p[:name] == key.to_sym
end.each do |p|
cond.add("#{key} = ?", value == "true")
end
end
options.merge!(cond.to_hash) unless cond.empty?
end
sferik
November 5, 2009, November 05, 2009 23:29, permalink
No rating.
Login to rate!
This is a nice idea, but to_cond isn't quite doing the right thing.
If params[:query] is "David" and @properties includes two string types, name and position, then after going through merge_query!, options should include:
{:conditions => ["name LIKE ? OR position LIKE ?", "%David%", "%David%"]}
...which then gets passed into merge_filter! and should become something like...
{:conditions => ["name LIKE ? OR position LIKE ? AND retired = ?", "%David%", "%David%", true]}
Does that make it any clearer how the current code works?
This is some controller code from MerbAdmin. An options hash is passed through these two methods, which add conditions to the hash if a query or filter params are specified. There is a lot of similar code between the two methods, which seems it could be factored into a single or common method.
If you want to verify that your refactoring doesn't break anything, you can checkout MerbAdmin on GitHub (http://github.com/sferik/merb-admin) and run rake. If I use your refactored code, your name will be added to the project's README. :)