Rubocop 指标/循环复杂度，以防

Question

指标/循环复杂度：notification_path 的循环复杂度太高。[9/8] 我不明白如何解决这个问题。有人可以帮我吗？谢谢

def notification_path
  return h.company_trial_activation_index_path if User.current.company.trial_pending?

  dot = object.eventable

  case object.event.to_sym
  when :new_app, :updates_approved, :updates_disapproved, :new_partial_app, :completed_app, :received_lead
    h.company_dot_application_path(id: dot.id)
  when :bg_check_received, :bg_check_failed, :bg_check_completed
    h.company_dot_application_background_checks_path(dot)
  when :voe_completed, :voe_corrected, :voe_invalid,
    :voe_driving_school_completed, :voe_driving_school_corrected, :voe_driving_school_invalid
    h.company_dot_application_applications_requests_path(dot_application_id: dot.id)
  when :voe_instructions, :voe_driving_school_instructions
    h.company_dot_application_applications_requests_path(dot.id)
  when :email_reply, :note, :sms_reply
    h.company_dot_application_communications_path(dot.id)
  when :follow_up_task, :follow_up_task_on_due_date
    h.company_dot_application_follow_up_tasks_path(dot_application_id: dot.id)
  when :bulk_actions_background_location, :bulk_actions_background_assign_user, :bulk_actions_background_lead_source
    h.home_users_path
  else
    h.home_users_path
  end
end

Answer 1

只是评论说我真的不同意@Schwern 的回答。

此 rubocop 规则的目的是提高可读性，因为高圈复杂度通常表明可读性较差（但并非总是如此）。

我认为你的语句可以重构和改进，但是 switch-case 模式在这里比元编程更合适！

元编程会混淆该代码的业务逻辑，并使以后的重构变得更加困难。

针对复杂性，关键是简单，而不是更加复杂:)。

我没有完整的背景，但我有很多线索：

• 尝试重构你的when条件。每一项都应对应一个具体且清晰的业务案例
• company_dot_application_applications_requests_path界面似乎不是最佳的。请记住，清晰的类和模块接口（方法、常量等）是干净代码的关键点之一
• 删除最后一个when（无用）

Answer 2

在讨论圈复杂度之前，让我们先解决该函数的两个更大问题。巨大的数组列表和全局变量。因为 Rubocop 的目的是让您的代码更易于阅读和维护，并减少出现错误的可能性，而不是在列表中勾选一些内容。

如果符号列表被命名，事情会更容易阅读。

WHATEVER_THESE_ARE = [
  :new_app, :updates_approved, :updates_disapproved,
  :new_partial_app, :completed_app, :received_lead
].freeze
BG_CHECK_DONE = [
  :bg_check_received, :bg_check_failed, :bg_check_completed
].freeze
...and so on...

def notification_path
  return h.company_trial_activation_index_path if User.current.company.trial_pending?

  dot = object.eventable

  case object.event.to_sym
  when WHATEVER_THESE_ARE
    h.company_dot_application_path(id: dot.id)
  when BG_CHECK_DONE
    h.company_dot_application_background_checks_path(dot)
  ...and so on...
  end
end

然后请注意您的函数不带参数。它使用两个全局变量，名称模糊的object和h。我不知道这些到底是什么，这些名字没有帮助，但也许你为了这个问题对它们进行了净化。使用它们作为全局变量会使代码更加复杂。它们应该作为参数传入，或者至少具有描述性名称。

def notification_path(better_name_for_h, better_name_for_object)
  ...
end

好的，接下来谈谈气旋的复杂性。Acase基本上是一个很大的 if/elsif/else，每个都when增加了一点复杂性。对于这么大的表，我们似乎无能为力，但我们首先观察一下情况仅取决于object.event、object.eventable（事件表？为什么命名为dot？） 和h。我们可以做一个提取方法。

def notification_path(h, object)
  return h.company_trial_activation_index_path if User.current.company.trial_pending?

  dot_to_application_path(object.event, object.eventable, h)
end

def dot_to_application_path(event, dot, h)
  case event.to_sym
  when WHATEVER_THESE_ARE
    h.company_dot_application_path(id: dot.id)
  when BG_CHECK_DONE
    h.company_dot_application_background_checks_path(dot)
  ...
  else
    h.home_users_path
  end
end

总共将 1 去掉，得到 8。其他人观察到最终条件是多余的，所以将其去掉，得到 7。有时，你比 Rubocop 更清楚，逻辑确实如此复杂，可以添加例外。

# rubocop:disable Metrics/CyclonicComplexity
def dot_to_application_path(event, dot, h)

这是最后的手段。Rubocop 的抱怨总是值得考虑的。通过尝试修复 Rubocop 问题，我们已经对代码进行了相当多的改进。

或者我们可以进一步挖掘。

我们可以观察到这张表正在做两件事。它选择适当的路径方法并规范其调用方式。每种路径方法都略有dot不同，这使得事情变得更加复杂并且容易出错。

如果方法一致，我们可以用哈希查找替换大小写。

WHATEVER_THESE_ARE = [
  :new_app, :updates_approved, :updates_disapproved,
  :new_partial_app, :completed_app, :received_lead
].freeze
BG_CHECK_DONE = [
  :bg_check_received, :bg_check_failed, :bg_check_completed
].freeze
...

# You'll want to put this somewhere else.
def flatten_keys(hash)
  hash.each_with_object({}) { |(keys, value), new_hash|
    keys.each { |key| new_hash[event] = value }
  }
end  

EVENT_TO_APPLICATION_PATH_METHOD = flatten_keys({
  WHATEVER_THESE_ARE => :company_dot_application_path,
  BG_CHECK_DONE => :company_dot_application_background_checks_path,
  ...
}).freeze

def notification_path(h, object)
  return h.company_trial_activation_index_path if User.current.company.trial_pending?

  method = EVENT_TO_DOT_APPLICATION_PATH_METHOD[object.event.to_sym] || :home_users_path
  h.send(method, dot)
end

现在我们可以看到这段代码是多么简单！

为了实现这一点，您可以重构这些方法，使它们保持一致，或者您可以编写使其一致的包装方法。重构是更好的选择，但这可能超出了您的范围。

最后，这可能足够复杂，需要将所有逻辑放入一个类中。