Rubocop度量/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。我不知道这些到底是什么，这些名字没有帮助，但也许你为了这个问题对它们进行了消毒。使用它们作为全局变量会使代码更加复杂。它们应该作为参数传递，或者至少是描述性的命名。

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

其他人观察到最后的条件是多余的，所以去掉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