Прежде чем мы перейдем к цикломатической сложности, давайте рассмотрим две большие проблемы с этой функцией.Гигантские списки массивов и глобалов.Потому что цель 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:disable Metrics/CyclonicComplexity
def dot_to_application_path(event, dot, h)
Это последнее средство.Всегда стоит рассмотреть жалобы Рубокопа.Мы уже немного улучшили код, пытаясь исправить проблемы с Rubocop.
Или мы можем копать дальше.
Мы можем заметить, что эта таблица выполняет two вещи.Он выбирает подходящий метод пути и , нормализуя способ его вызова.Каждый метод пути воспринимает 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
Теперь мы можем видеть, насколько простым может быть этот код!
Для этого вы можете реорганизовать методы, чтобы они были согласованными, или вы можете написать методы-оболочки, которые сделают его согласованным.Рефакторинг является предпочтительным вариантом, но это может быть далеко за пределами вашей области.
Наконец, все это может быть достаточно сложным, чтобы оправдать помещение всей логики в класс.