ruby-on-rails - Rubocop Metrics/CyclomaticComplexity 以防万一

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

在讨论圈复杂度之前，让我们解决该函数的两个更大的问题。巨大的数组列表和全局变量。因为 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。我不知道这些到底是什么，这些名字没有帮助，但也许你为这个问题对它们进行了 sanitizer 。将它们用作全局变量会使代码更加复杂。它们应作为参数传入，或至少以描述性命名。

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 更清楚逻辑真的很复杂并且可以添加一个异常(exception).

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

这是最后的手段。考虑 Rubocop 的提示总是值得的。我们已经通过尝试修复 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
...

# 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

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

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

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