对参数的许多if语句进行重构[…]，控制器内动作

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

如你所见，我的控制器代码中有太多的if。我要在这里添加更多的条件。在那之后，任何读到代码的人都会大脑腐败。所以我只是想用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和fields_for来简化这一点，并完全摆脱助手，但这取决于你的模型关联是如何设置的。

Answer 2

对于这种问题没有明显的解决办法。

一般来说，我尽量保持if/else架构非常清晰，并将所有代码导出到单独的方法中。这里有两个优点:

可读性

• 更容易进行单元测试

对于您的情况，它将是:

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