Rubocop Metrics/CyclomaticComplexity 以防万一

Question

Metrics/CyclomaticComplexity：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

在我们讨论圈复杂度之前，让我们解决这个函数的两个更大的问题。巨大的数组列表和全局变量。因为 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

---

好的，关于气旋复杂性。 case 基本上是一个很大的 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

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

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

最后，这可能都足够复杂，足以保证将所有逻辑放入一个类中。

Answer 2

我注意到你使用

h.company_dot_application_applications_requests_path(dot_application_id: dot.id)

在一个地方和

h.company_dot_application_applications_requests_path(dot.id)

在另一个。如果这些调用是相等的，方法可以简化一点：

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, :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)
  else
    h.home_users_path
  end
end