Ruby on Rails: refactor big search method

Question

I have big search method in my model that corresponds to search a proper Car.

It looks like this:

 def self.search(params) 
 cars = joins(:reservations).where.not("reservations.reception_time <= ? AND reservations.return_time >= ?", 
 params[:return_date], params[:handover_date])
 cars = joins(:car_class).where("car_classes.id= ?", params[:car_class])
 cars = cars_at_both_locations(params[:handover_location], params[:return_location])
 cars = params[:car_body_style] == [""] ? cars : joins(:car_configuration).
 where("car_configurations.body_style_id = ?", params[:car_body_style]) 
 cars = params[:car_fuel] == [""] ? cars : where(fuel: params[:car_fuel]) 
 cars = params[:car_transmission] == [""] ? cars : where(transmission: params[:car_transmission]) 
 cars = params [:car_seats] == [""] ? cars : car_seats(params[:car_seats])
 cars = Car.joins(:prices).where('prices.to_days >= ?', 
 (Date.parse(params[:return_date]) - Date.parse(params[:handover_date])).to_i)
end

It is very unreadable now.

Does anyone have an idea how to refactor this method to be more readable?

• tokland: Does this help? stackoverflow.com/a/4480285/188031
• Mateusz: Yes, it helps a lot :)

Answer 1

Some notes on your code:

• As you know, in Rails, params usually refers to query strings in a request. In a model I'd use something like attributes or simply options to avoid confussion.
• Variable cars reused multiple times: yes, it's a very usual pattern, but IMO it's not right. I think different values deserve different names.
• You have a pattern to abstract here, use inject.

I gave an answer in SO a while ago with exactly this kind of problem, that's the approach I'd recommend: Approach post in SO