如何重构这个6行方法以使其更具可读性？

Question

我正试图在这里清理这个非常丑陋的方法,那是为了重构而哭泣,但我不确定哪种结构能做到最好(即一个案例陈述,或者只是一个精心设计的if then陈述)

乍一看,它似乎是一个案例陈述的理想场所,有一些很好的位置when,但我的理解是案例陈述只能用于单个变量,而不是两个,以及各种带有irb的小提琴使用散列或数组来尝试这些语句在这里也没有太多亮点.

你会怎么做？在检查这样的多个布尔值时,Ruby中是否有任何常见的技巧来避免这样的代码？

  def has_just_one_kind_of_thing?(item, controller)
    if (controller == 'foos' && item.widgets.blank?) || (controller == 'foos' && item.doohickeys.blank?) || (controller == 'bars' && item.widgets.blank?) || (controller == 'bars' && item.doohickeys.blank?) || (controller == 'bazes' && item.widgets.blank?) || (controller == 'bazes' && item.contraptions.blank?)
      return true
    else 
      return false
    end
  end

Answer 1

伙计们 - 这里的多态性似乎需要通过在一种方法中摆弄流量控制来解决.

这是我从扩展版本开始的尝试:

def has_just_one_kind_of_thing?(item, controller)
  (controller == 'foos' && item.widgets.blank?)    || 
  (controller == 'foos' && item.doohickeys.blank?) || 
  (controller == 'bars' && item.widgets.blank?)    || 
  (controller == 'bars' && item.doohickeys.blank?) || 
  (controller == 'bazes' && item.widgets.blank?)   || 
  (controller == 'bazes' && item.contraptions.blank?)
end

因此,有三种不同的行为 - 每个控制器一个 ....如果每个控制器足够智能以包含特定于控制器的决策,并且您传入实际控制器而不仅仅是控制器的名称,则将方法减少到:

def has_just_one_kind_of_thing?(item, controller)
  controller.has_just_one_kind_of_thing?(item)
end

这要求每个控制器对其所属的控制器进行相关的项目处理.因此,让我们在每个foos,bars和bazes上定义一个方法has_just_one_kind_of_thing?

foos的例子:

def has_just_one_kind_of_thing?(item)
   item.widgets.blank? || item.doohickeys.blank?
end

bazes的例子:

def has_just_one_kind_of_thing?(item)
   item.widgets.blank? || item.contraptions.blank?
end

在要返回的每个控制器false上,只需应用"常量方法"模式:

def has_just_one_kind_of_thing?(item)
  false
end

这段代码甚至会运行得更快,因为现在,我们不需要像控制器类型那样进行多次检查 - 我们只需要对控制器进行一次方法调度.

因此它在解释红宝石方面更快 - 它甚至可以在jruby或其他一种可以大大优化方法调度的红宝石中快得多 ......

我们可能会让它变得更聪明,但我需要知道这种方法所处的类别,以及其他一些事情item.

另一种重构可能就是变得item聪明,并且对每种类型都有不同的方法item.同样,我们需要了解更多关于对象模型的信息,以告诉哪个是最好的......

仍然是第一次削减.

Answer 2

也许是这样的东西？

def has_just_one_kind_of_thing?(item, controller)
    return case controller
      when 'foos', 'bars'
        item.widgets.blank? || item.doohickeys.blank?
      when 'bazes'
        item.widgets.blank? || item.contraptions.blank?
      else 
        false
    end
  end

外部回归可能没有必要(不完全确定Ruby需要什么,我自己还是相当新的),但我更喜欢把它放进去,所以意图很明显.