如何重构这组庞大的 if 语句？

Posted 2023-02-15

【中文标题】如何重构这组庞大的 if 语句？

【英文标题】：How do I refactor this bulky set of if statements?

【发布时间】：2019-05-04 06:22:33

【问题描述】：

我有以下有效的代码，但我希望它更干爽、更优雅。感觉很丑，有很多代码味道。

理想情况下，我不想使用 CASE 语句，因为这也不太像 ruby​​ 风格。

if @property_status.eql? :rent
  if @property_type.eql? :residential
    @results = @search.results.for_rent.residential.order("# @sort_by  # @sort_order ").all.paginate(page: @page, per_page: @per_page)
  elsif @property_type.eql? :commercial
    @results = @search.results.for_rent.commercial.order("# @sort_by  # @sort_order ").all.paginate(page: @page, per_page: @per_page)
  else
    @results = @search.results.for_rent.order("# @sort_by  # @sort_order ").all.paginate(page: @page, per_page: @per_page)
  end
elsif @property_status.eql? :sale
  if @property_type.eql? :residential
    @results = @search.results.for_sale.residential.order("# @sort_by  # @sort_order ").all.paginate(page: @page, per_page: @per_page)
  elsif @property_type.eql? :commercial
    @results = @search.results.for_sale.commercial.order("# @sort_by  # @sort_order ").all.paginate(page: @page, per_page: @per_page)
  else
    @results = @search.results.for_sale.order("# @sort_by  # @sort_order ").all.paginate(page: @page, per_page: @per_page)
  end
else
  @results = @search.results.order("# @sort_by  # @sort_order ").all.paginate(page: @page, per_page: @per_page)
end

想法？

【问题讨论】：

我投票结束这个问题，因为它更适合Code Review。

【参考方案1】：

首先认识到这一点：

@results = @search.results.for_rent.residential.order("# @sort_by  # @sort_order ").all.paginate(page: @page, per_page: @per_page)

相当于：

@results = @search.results
@results = @results.for_rent
@results = @results.residential
@results = @results.order(@sort_by => @sort_order).paginate(page: @page, per_page: @per_page)

假设@sort_by 和@sort_order 总是设置为当然。重要的是您可以逐个构建查询，并根据您的实例变量选择要添加的部分。您可以添加几个简单的助手：

def add_property_status_to(query)
  case @property_status
  when :rent, :sale
    query.public_send("for_#@property_type")
  else
    query
  end
end

def add_property_type_to(query)
  case @property_type
  when :residential, :commercial
    query.public_send(@property_type)
  else
    query
  end
end

然后这样说：

query = @search.results
query = add_property_status_to(query)
query = add_property_type_to(query)

@results = query.order(@sort_by => @sort_order).paginate(page: @page, per_page: @per_page)

您可以将add_property_status_to 和add_property_type_to 方法视为本地单次使用范围。如果您需要在多个地方使用它们，那么您可以将它们作为 @search.results 的类方法，然后说：

query = @search.results
query = query.with_property_status(@property_status)
query = query.with_property_type(@property_type)

@results = query.order(@sort_by => @sort_order).paginate(page: @page, per_page: @per_page)

【参考方案2】：

我没有测试它，但我认为是这样的：

def results
  return results_for_rent if for_rent?
  return results_for_sale if for_sale?
  default_results
end

def for_rent?
  @property_status.eql? :rent
end

def for_sale?
  @property_status.eql? :sale
end

def default_results
  @results = @search.results.order("# @sort_by  # @sort_order 
   ").all.paginate(page: @page, per_page: @per_page)
end

def results_for_rent
  if [:residential, :commercial].includes?(@property_type)
    results(@property_type, type: :for_rent)
    return 
  end
  @results = @search.results.for_rent.commercial.order("# @sort_by  # 
  @sort_order ").all.paginate(page: @page, per_page: @per_page)
end

def results_for_sale
  if [:residential, :commercial].includes?(@property_type)
    results(kind, type: :for_sale)
    return
  end
  @results = @search.results.for_sale.order("# @sort_by  # @sort_order 
  ").all.paginate(page: @page, per_page: @per_page)
end

def results(kind:, type: )
  @results = @search.results.send(kind).send(type).order("# @sort_by  # 
  @sort_order ").all.paginate(page: @page, per_page: @per_page)
end

您也可以重构相同的 .all.pagination 部分，但您明白了。您还可以通过添加一个参数来重构 results_for_rent 和 results_for_sale 方法

【讨论】：

谢谢你，虽然这感觉它比原来的更难读和更难理解。所以我觉得我为半干性牺牲了可理解性。

它更容易理解，因为：results 方法只有 3 个条件。

请注意，Rails 添加了Object#in?，因此您可以说@property_type.in?(%i[residential commercial]) 之类的内容，通常比等效的#include? 版本更容易阅读。