为 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

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

有趣的是，在 all 这之后，我的整个 class 现在被 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" 部分都消失了。现在一切都非常明确，出现模糊小错误的机会更少了。

当然，如果您在 knows/likes 函数式风格的团队中工作，则只做这样的事情；有些人喜欢它，有些人讨厌它。

如果你没有在 Hash 和 Array 上工作，我可能会建议使用 OO 而不是函数（即，将两种处理方法分配到它们各自的类 ). re-opening 哈希和数组技术上可行；不过当然不可取。

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

Refactoring Long and Too Many Branches Methods for Rubocop

ruby

rubocop