У меня есть следующий метод в моем тестовом приложении:
def on(definition, visit = false, &block)
if @page.is_a?(definition)
block.call @page if block
return @page
end
if @context.is_a?(definition)
block.call @context if block
@page = @context unless @page.is_a?(definition)
return @context
end
@page = definition.new(@browser)
@page.view if visit
@page.correct_url? if @page.respond_to?(:url_matches)
@page.correct_title? if @page.respond_to?(:title_is)
@model = @page
block.call @page if block
@page
end
Когда я запускаю инструмент rubocop для файла, содержащего этот метод, я получаю следующие ответы:
C: Cyclomatic complexity for on is too high. [10/6]
C: Perceived complexity for on is too high. [10/7]
Чего я не понимаю, так это того, что он считает «слишком сложным», поэтому мне трудно понять, как решить проблему. В идеале я бы предпочел не просто говорить rubocop, чтобы он избегал предупреждения, так как он, без сомнения, сообщает мне что-то полезное.
Что касается сложности метода, как видите, у меня есть пара вызовов if
, а затем мне нужно поработать с объектом @page
, чтобы убедиться, что он настроен правильно. (Этот пример в контексте представляет собой объект Watir-WebDriver.)
Я согласен, что метод сложен в том смысле, что он требует проверки того, существует ли уже @page
и установлено ли что-то, а также проверки того, должно ли @page
совпадать с @context
. Но -- опять же, я не знаю, что с этим делать.
Полный код модуля, внутри которого находится этот метод, находится здесь:
https://github.com/jnyman/symbiont/blob/master/lib/symbiont/factory.rb
Сначала я думал, что могу просто разбить это на разные вызовы методов, что может уменьшить сложность каждого метода. Но тогда это означает, что кто-то, кто читает мой код, должен использовать ряд различных методов, чтобы понять, что делает on
. Мне кажется, что простое перемещение вещей не устраняет сложность в целом; скорее, это было бы просто перетасовкой. Или я ошибаюсь?
Любые советы здесь ценятся.
ОБНОВЛЕННЫЙ КОД
У меня это несколько уменьшилось. Вот что у меня сейчас есть:
def on(definition, visit = false, &block)
if @page.is_a?(definition)
block.call @page if block
return @page
end
if @context.is_a?(definition)
block.call @context if block
@page = @context
return @context
end
@page = definition.new(@browser)
@page.view if visit
@model = @page
block.call @page if block
@page
end
Основываясь на отзывах, я удалил квалификатор unless
, который казался бесполезным. Я также удалил две строки, которые, как мне показалось, можно было бы лучше использовать в другом месте (проверив заголовок и URL-адрес).
Это полностью устранило «воспринимаемую сложность» и оставило мне только это:
C: Cyclomatic complexity for on is too high. [7/6]
Мне кажется, что "одна точка" (или какая там терминология) слишком сложна.
on
вызывается так:on(SomePage)
. Это может произойти в любом месте тестового сценария. Кроме того, вы можете иметьon(SomePage)
, а затемon(SomeOtherPage)
. Для логики важно знать, что SomeOtherPage не является SomePage. Но вы также можете сделатьon_new(SomePage)
, что означает, что SomePage — это SomePage, но новый — другой — экземпляр. Итак, проблема в том, что я не знаю, что собирается делать автор тестов. Тем не менее, приняв ваши слова близко к сердцу, я посмотрю, смогу ли я его очистить. - person Jeff Nyman   schedule 03.10.2015