1

我的测试应用程序中有以下方法：

def on(definition, visit = false, &block)
  if @page.is_a?(definition)
    block.call @page if block
    return @page
  end

  if @context.is_a?(definition)
    block.call @context if block
    @page = @context unless @page.is_a?(definition)
    return @context
  end

  @page = definition.new(@browser)
  @page.view if visit

  @page.correct_url? if @page.respond_to?(:url_matches)
  @page.correct_title? if @page.respond_to?(:title_is)

  @model = @page

  block.call @page if block

  @page
end

当我对包含此方法的文件运行 rubocop 工具时，我得到以下响应：

C: Cyclomatic complexity for on is too high. [10/6]
C: Perceived complexity for on is too high. [10/7]

我没有得到它认为“太复杂”的东西，因此我无法弄清楚如何解决这个问题。理想情况下，我宁愿不只是告诉 rubocop 避免警告，因为毫无疑问它告诉我一些有用的东西。

正如您所看到的，就复杂的方法而言，我有几个if调用，然后我必须使用该@page对象以确保它设置正确。（这个例子，在上下文中，是一个 Watir-WebDriver 对象。）

我确实同意该方法很复杂，因为它需要检查是否@page已经存在并设置为某物，以及检查是否@page应该与@context. 但是——再说一次，我不知道该怎么办。

该方法所在模块的完整代码在这里：

https://github.com/jnyman/symbiont/blob/master/lib/symbiont/factory.rb

我最初认为我可以将其分解为不同的方法调用，这可能会降低每种方法的复杂性。但这意味着阅读我的代码的人必须跳到一系列不同的方法来理解on正在做什么。在我看来，仅仅移动东西似乎并不能消除整体的复杂性。相反，它只是把它洗牌。还是我错了？

任何建议在这里表示赞赏。

更新代码

我已经减少了一些。这是我现在拥有的：

def on(definition, visit = false, &block)
  if @page.is_a?(definition)
    block.call @page if block
    return @page
  end

  if @context.is_a?(definition)
    block.call @context if block
    @page = @context
    return @context
  end

  @page = definition.new(@browser)
  @page.view if visit

  @model = @page

  block.call @page if block

  @page
end

根据反馈，我删除了一个unless似乎确实没用的限定符。我还删除了两行我发现可以在其他地方更好地使用的行（检查标题和网址）。

这完全消除了“感知的复杂性”，只剩下了这个：

C: Cyclomatic complexity for on is too high. [7/6]

我似乎“一点”（或任何术语）太复杂了。

4

1 回答 1

2

有些地方你的代码是多余的。例如，你有这个：

if @page.is_a?(definition)
  ...
  return ...
end

这意味着，在遵循此部分的任何部分，您都可以假设@page.is_a?(definition)不是true，除非您@page在此部分之后进行修改。尽管如此，你有：

if @context.is_a?(definition)
  ... unless @page.is_a?(definition)
end
于 2015-10-03T14:02:23.150 回答