2016-12-08 4 views
1

クエリを実行する前に、空のパラメータのチェックを行っています。 params [:car_model_id]のチェックは1つだけです。他のパラメータのチェックを追加すると、if-else文が混乱することが想像できます。それは素晴らしく見えず、最適化できると思います。しかしどうですか?ここで、コントローラのコードは次のとおり空のパラメータのチェックが多すぎます。 Rails5でActiveRecordへのクエリを最適化する方法は?

class CarsController < ApplicationController 
def search 
    if params[:car_model_id].empty? 
    @cars = Car.where(
     used: ActiveRecord::Type::Boolean.new.cast(params[:used]), 
     year: params[:year_from]..params[:year_to], 
     price: params[:price_from]..params[:price_to], 
     condition: params[:condition] 
    ) 
    else 
    @cars = Car.where(
     used: ActiveRecord::Type::Boolean.new.cast(params[:used]), 
     car_model_id: params[:car_model_id], 
     year: params[:year_from]..params[:year_to], 
     price: params[:price_from]..params[:price_to], 
     condition: params[:condition] 
    ) 
    end 

    if @cars 
    render json: @cars 
    else 
    render json: @cars.errors, status: :unprocessable_entity 
    end 
end 
end 

答えて

1

トリックは、ブランク値を削除データの前処理の少しを行う(そしておそらくバリデーション)、次いでwhere句へのparamsを通過することであろう。日付範囲の処理を支援する

、あなたは両方の日付が提供されており、範囲に変換されているチェックの方法で作成できます

def convert_to_range(start_date, end_date) 
    if start_date && end_date 
    price_from = Date.parse(price_from) 
    price_to = Date.parse(price_to) 
    price_from..price_to 
    end 
rescue ArgumentError => e 
    # If you're code reaches here then the user has invalid date and you  
    # need to work out how to handle this. 
end 

を次に、あなたのコントローラのアクションは、次のようなものを見ることができます

# select only the params that are need 
car_params = params.slice(:car_model_id, :used, :year_from, :year_to, :price_from, :price_to, :condition) 

# do some processing of the data 
year_from = car_params.delete(:year_from).presence 
year_to = car_params.delete(:year_to).presence 
car_params[:price] = convert_to_range(year_from, year_to) 

price_from = car_params.delete(:price_from).presence 
price_to = car_params.delete(:price_to).presence 
car_params[:price] = convert_to_range(price_from, price_to) 

# select only params that are present 
car_params = car_params.select {|k, v| v.present? } 

# search for the cars 
@cars = Car.where(car_params) 

はまた、私はused値が自動的にwhereに提供された場合、あなたのためにブール型にキャストを取得することをかなり確信しています。

また、@carsはメソッドを持たないActiveRecord::Relationです。おそらく、返される車があるかどうかに基づいて異なる結果を出すことを意味しますか?

例:@cars.any?(または@cars.load.any?あなたが車をフェッチし、車が存在するかどうかを確認するために、2つのクエリを実行したくない場合)

編集:

あなたはクリーンもmu is too shortによって可能で述べたようにwhere条件とscopesを連鎖させてコードを作成してください。スコープは、コントローラからモデルに機能を移すのに役立ち、機能の再利用性を向上させます。

など。

コントローラ内で次に
class Car > ActiveRecord::Base 

    scope :year_between, ->(from, to) { where(year: from..to) } 
    scope :price_between, ->(from, to) { where(price: from..to) } 

    scope :used, ->(value = true) { where(used: used) } 

end 

:( 'B'、C 'D') `` M.whereと同じである

# initial condition is all cars 
cars = Cars.all 

# refine results with params provided by user 
cars = cars.where(car_model_id: params[:car_model_id]) if params[:car_model_id].present? 
cars = cars.year_between(params[:year_from], params[:year_to]) 
cars = cars.price_between(params[:price_from], params[:price_to]) 
cars = cars.used(params[:used]) 
cars = cars.where(condition: params[:condition]) if params[:condition].present? 

@cars = cars 
+1

も ​​'M.whereは何の価値もないかもしれません(a: 'b')。ここで(c: 'd') 'または' ms = M. ms = ms.where(c: 'd') 'のように、' if'と 'else'ブランチの共通部分を簡単に取り除くことができます。あなたがスコープを投げることができるように、ハッシュを操作するよりも少し柔軟です。 –

+0

私は '' 'car_params.delete''に固執しています レールからデバッグコンソール:' '' 2.2.3:035> car_params [:年] = car_params.delete(:year_from).. car_params.delete (:year_to) => nil..nil 2.2.3:037> Car.where(car_params) 車の負荷(0.1ms)SELECT「cars」。* FROM「cars」WHERE「cars」。「car_model_id」= ? AND "cars"。 "used" =? AND "cars"。 "condition" =? AND( "cars"。 "年"?AND?)AND( "cars"。 "price"?)、["条件"、 "完璧"]、[年]、[なし]、[年]、[なし]、[価格]、[無制限] ]、["price"、nil]] =># '' ' – Freeminder

+0

リクエストに空のパラメータがあります:' '' year_from "=" "、" year_to "=> "、" price_from "=>" "" price_to "=>" "' '' そして.deleteメソッドはそれを '' '..''に変換します。 – Freeminder

関連する問題