重構如果報表PARAMS [...]，控制器動作裏面

Question

我有這樣的代碼，用於製造鏈選擇在我的形式 查看索引行動：

<%= form_tag do %> 
    <%= collection_select(*@brands_select_params) %> 
    <%= collection_select(*@car_models_select_params) %> 
    <%= collection_select(*@production_years_select_params) %> 
    <% # Пока еще никто ничего не выбрал %> 
<%= submit_tag "Send", :id => "submit", :name => "submit" %> 

而且我的控制器：

class SearchController < ApplicationController 
    def index 
    @brands = Brand.all 
    @car_models = CarModel.all 

    if (params[:brand].blank?) 
     @brands_select_params = [:brand, :id, @brands, :id, :name, :prompt => "Выбирай брэнд"] 

     if params[:car_model].blank? 
     @car_models_select_params = [:car_model, :id, @car_models, :id, :name, { :prompt => "Model" }, \ 
            { :disabled => "disabled" }] 
     @production_years_select_params = [:production_year, :id, @car_models, :id, :name, { :prompt => "Year" }, \ 
             { :disabled => "disabled" }] 
     end 
    else 
     @brands_select_params = [:brand, :id, @brands, :id, :name, { :selected => params[:brand][:id] } ] 
     if params[:car_model].blank? 
     @car_models_select_params = [:car_model, :id, Brand.find(params[:brand][:id]).car_models, :id, :name, \ 
            { :prompt => "And model now" } ] 
     @production_years_select_params = [:production_year, :id, @car_models, :id, :name, { :prompt => "Year" }, \ 
             { :disabled => "disabled" } ] 
     else 
     @car_models_select_params = [:car_model, :id, Brand.find(params[:brand][:id]).car_models, :id, :name, \ 
            { :selected => params[:car_model][:id] } ] unless params[:car_model][:id].empty? 
     @production_years_select_params = [:production_year, :id, CarModel.find(params[:car_model][:id]).production_years, :id, :year, \ 
             { :prompt => "And year now" } ] unless params[:car_model][:id].empty? 
     end 
    end 
    end 
end 

正如你所看到的，我的控制器代碼中有太多ifs。我會在那裏添加更多的條件。之後，任何閱讀該代碼的人都會得到腦部腐敗。所以我只是想用真正的Ruby方式來製作它，但不知道如何。請幫忙，夥計們。我應該如何重構這堆廢話？

Answer 1

我認爲這個問題的一個重要組成部分你在控制器中做得太多了嗎？生成標記（以及包含用於表單助手的構建參數列表的IMO）應該在視圖和視圖助手中完成。所以：

module SearchHelper 
    def brand_select brands, options={} 
    collection_select :brand, :id, brands, :id, :name, :options 
    end 

    def car_model_select car_models, options={} 
    collection_select :car_model, :id, car_models, :id, :name, options 
    end 

    def production_year_select years, options={} 
    collection_select :production_year, :id, years, :id, :year, options 
    end 
end 

然後，你可以降低你的控制器到這一點：

def index 
    @brands  = Brand.all 
    @car_models = CarModel.all 

    @selected_brand_id  = params[:brand]  && params[:brand][:id] 
    @selected_car_model_id = params[:car_model] && params[:car_model][:id] 

    @production_years = @selected_car_model_id ? 
    [] : CarModel.find(@selected_car_model_id).production_years 
end 

而在你的看法：

<%= brand_select @brands, :prompt => "Выбирай брэнд", 
          :selected => @selected_brand_id 
%> 
<%= car_model_select @car_models, :prompt => "Model", 
            :selected => @selected_car_model_id 
%> 
<%= production_year_select @production_years, :prompt => "Year", 
               :selected => @selected_car_id 
%> 

我懷疑，你可以簡化這個更使用form_for and fields_for，並得到完全擺脫助手，但這取決於你的模型關聯是如何建立的。

Answer 2

這種問題沒有明顯的解決方案。

通常，我會盡量保持if/else架構非常清晰，並將所有代碼導出到不同的方法中。優點2瀏覽：

• 可讀性

• 方便單元測試

對於你的情況，那就是：

class SearchController < ApplicationController 
    def index 
    @brands = Brand.all 
    @car_models = CarModel.all 

    if (params[:brand].blank?) 
     @brands_select_params = [:brand, :id, @brands, :id, :name, :prompt => "Выбирай брэнд"] 
     if params[:car_model].blank? 
     @car_models_select_params, @production_years_select_params = get_card_model(@car_models) 
     end 
    else 
     @brands_select_params = [:brand, :id, @brands, :id, :name, { :selected => params[:brand][:id] } ] 
     if params[:car_model].blank? 
     @car_models_select_params, @production_years_select_params = foo_method(@car_models) 
     else 
     @car_models_select_params, @production_years_select_params = bar_method 
     end 
    end 
    end 

    def get_card_model car_models 
    [ 
    [:car_model, :id, car_models, :id, :name, { :prompt => "Model" }, { :disabled => "disabled" }], 
    [:production_year, :id, car_models, :id, :name, { :prompt => "Year" }, { :disabled => "disabled" }] 
    ] 
    end 
end