重构 Rubocop 的长分支和过多分支方法

Question

我在下面提供了一些发现的更新...

这是我当前代码中的一种方法：

def query(data_set, conditions)
  query_data_set = data_set.dup
  search_conditions = parse_conditions(conditions)

  search_conditions.each do |condition|
    if condition.class == Array
      condition.each do |term|
        if term.class == Symbol
          query_data_set = entity.send term, query_data_set
        else
          query_data_set = search_data_text(query_data_set, term)
        end
      end
    end

    if condition.class == Hash
      condition.each do |key, value|
        query_data_set = method("query_#{key}").call(data_set, value)
      end
    end
  end

  query_data_set
end

Rubocop 讨厌这个有三个原因：

C: Assignment Branch Condition size for query is too high. [17.03/15]
  def query(data_set, conditions)

C: Method has too many lines. [19/7]
  def query(data_set, conditions)

C: Use next to skip iteration.
  search_conditions.each do |condition|

我不想跳过迭代，所以我不知道为什么我会使用next。对于这段代码来说，跳过迭代永远不会有意义。

因此转到其他投诉，您会在此方法的顶部看到我已经展开了一个操作（对parse_conditions 的调用）。还有一个电话到search_data_text。我在这里唯一的一点是我试图在似乎有意义的地方进行模块化。

即使我将那个大的search_conditions.each 块移到一个单独的方法中，Rubocop 也会抱怨我的新方法太长了。我想这意味着添加我的 second 方法将调用的 third 方法？这对我来说似乎很奇怪。或者也许这意味着我不必分支太多，我猜。但是为什么分支不好？即使我切换到其他结构（比如案例......何时），我仍然在分支。而且我确实需要测试这些条件，因为嵌套数组、带有符号的数组或散列的处理是不同的。

我正在努力建立自己的直觉，以便能够看到这样的问题并找出有效且高效的解决方案....但似乎我最终做的事情是非常低效和无效与我的时间。考虑到我看不出我的代码为什么不好，这种权衡让我很担心。

任何人介意尝试一下并帮助我了解如何使上述方法进入一种保持一些可读性但符合 Ruby 主义者喜欢的样式指南的状态？

---------- 更新 ----------

我能想到的最好的方法是：

def query(data_set, conditions)
  query_data_set = data_set.dup
  parse_conditions(conditions).each do |condition|
    query_data_set = check_conditions(condition, query_data_set)
  end
  query_data_set
end

def check_conditions(condition, data)
  if condition.class == Array
    condition.each do |term|
      data = entity.send term, data if term.class == Symbol
      data = search_data_text(data, term) unless term.class == Symbol
    end
  end

  if condition.class == Hash
    condition.each do |key, value|
      data = method("query_#{key}").call(data, value)
    end
  end
  data
end

check_conditions 方法对于 Rubocop 来说仍然太长，并且分支条件大小仍然过高。

据我所见，我检查过的任何地方都无法向我展示不同的东西，唯一可以做的可能是从数组和哈希的检查中创建方法。换句话说，check_conditions 中的每个if 条件都会有自己的方法。但这对我来说似乎是不必要的愚蠢。我基本上是在分解逻辑，将变量传递给不同的方法，这样我就可以将我的方法计数保持在某个任意值以下。

作为一种设计方式，这对我来说是错误的。但是我看不到一种方法可以改变我的逻辑，使它仍然可以完成它需要做的事情，但每个方法不到七行。

Answer 1

def query(data_set, conditions)
  data_set = data_set.dup
  parse_conditions(conditions).each do |condition|
    condition.each do |e|
      case e
      when Array  then method("query_#{e.first}").call(data_set, e.last)
      when Symbol then entity.send(e, data_set)
      else             search_data_text(data_set, e)
      end
    end
  end
end

Answer 2

好的，找到方法了。它可能不是最干净的，但没有其他工作。这是我所做的：

def query(data_set, conditions)
  query_data_set = data_set.dup
  parse_conditions(conditions).each do |condition|
    query_data_set = check_conditions(condition, query_data_set)
  end
  query_data_set
end

def check_conditions(condition, data)
  data = process_condition_array(condition, data) if condition.class == Array
  data = process_condition_hash(condition, data) if condition.class == Hash
  data
end

def process_condition_array(condition, data)
  condition.each do |term|
    data = entity.send term, data if term.class == Symbol
    data = search_data_text(data, term) unless term.class == Symbol
  end
  data
end

def process_condition_hash(condition, data)
  condition.each do |key, value|
    data = method("query_#{key}").call(data, value)
  end
  data
end

所以我有一种我认为是合理的方法，尽管承认有点罗嗦。但一切都在一个地方。现在这已成为四种方法，作为代码的读者，您必须通过这些方法跟踪变量。我想这就是进步。 :-/

有趣的是，在所有这一切之后，Rubocop 认为我的整个课程现在太长了。 叹息。不管怎样，以那种特定的任意风格生活至少比其他任意风格的生活更可口。

Answer 3

为了扩展您自己的答案，我可能会建议一些功能优势...（即Enumerable.inject 加上lambda A.K.A. 一阶函数）。

（请注意，我没有质疑或更改您自己代码的逻辑；我只是机械地对其进行了转换）。

def query(data_set, conditions)
  parse_conditions(conditions).inject(data_set) do |data_set, condition|
    check_conditions(condition, data_set)
  end
end

processors = {
  'Array' => lambda do |condition, data|
               condition.inject(data) do |data,term|
                 term.class == Symbol ? entity.send(term, data)
                                      : search_data_text(data, term) 
               end
             end,
  'Hash' => lambda do |condition, data|
               condition.to_a.inject(data) do |data, pair|
                 method("query_#{pair[0]}").call(data, pair[1])
               end
             end
}

def check_conditions(condition, data)
  processors[condition.class.name].call(condition, data)
end

少了几行代码（无关紧要），显式的 if/else 控制结构加上显式的“data =”和“return data”部分都消失了。现在一切都非常明确，出现晦涩的小错误的机会更少了。

当然，只有当你在一个了解/喜欢功能风格的团队中工作时才这样做；有些人喜欢它，有些人讨厌它。

如果您不是在处理 Hash 和 Array，我可能会建议使用 OO 而不是函数（即将这两种处理方法分配到它们各自的类中）。技术上可以通过重新打开 Hash 和 Array；当然不建议这样做。