Метод рефакторинга с множеством условных операторов возврата - PullRequest
3 голосов
/ 02 марта 2010

У меня есть метод для проверки, который имеет много условных операторов. В основном это идет

If Check1 = false 
  return false
If Check2 = false
  return false
etc

FxCop жалуется, что цикломатическая сложность слишком высока. Я знаю, что не рекомендуется использовать операторы return в середине функций, но в то же время единственная альтернатива, которую я вижу, это ужасный список операторов If-else. Как лучше всего подойти к этому?

Заранее спасибо.

Ответы [ 4 ]

11 голосов
/ 02 марта 2010

Я не согласен с вашим утверждением, что не рекомендуется использовать операторы return в середине методов. Длина, к которой прибегают некоторые люди для того, чтобы иметь единственное выражение возврата, сумасшедшая - используйте все, что дает наиболее читаемый код. Иногда это будет единственная точка возврата, но часто я нахожу, что есть «ранний выход» - и лучше иметь такой возврат, чем вводить больше вложенности в код с if для альтернативного пути. Мне нравятся методы, которые не имеют слишком большого отступа, как правило:)

Сказав это, действительно ли метод ничего , но проверяет? Чеки независимы? Какие переменные они требуют? Можете ли вы сгруппировать их в более мелкие методы, представляющие «области» проверок, которые вы выполняете?

1 голос
/ 02 марта 2010

В вашем конкретном случае кажется, что вы, вероятно, могли бы использовать цикл ... предполагая, что это происходит из обработчика событий в форме или что-то в этом роде:

For Each ctrl As Control In Me.Controls

    Dim check As CheckBox = TryCast(ctrl, CheckBox)

    If check IsNot Nothing AndAlso check.Checked = False Then
        Return False
    End If

Next

Return True

Редактировать: После дальнейшего изучения я понимаю, что это основано на psuedocode, и вы фактически не использовали флажки. Однако , я думаю, что применяется та же методика. Вы можете очень легко реорганизовать ваши правила в отдельный класс, который реализует пользовательский интерфейс (IRule или аналогичный), или иметь список делегатов, которые возвращают истину / ложь, которые вы будете повторять как часть своего шаблона проверки.

1 голос
/ 02 марта 2010

Используя ваш пример:

return (Check1 == false)   ||   (Check2 == false)   [ || etc ]
0 голосов
/ 16 июня 2010

Если у вас есть несколько проверок подряд и вам не нужно сохранять короткое замыкание, вы можете попробовать что-то вроде этого:

        bool isValid = true;
        isValid &= Condition1;
        isValid &= Condition2;
        isValid &= Condition3;
        if (!isValid)
            return false;

Если вам нужно сохранить короткое замыкание, вы можете объединить условия в большой оператор if. Однако если условий много, я бы предпочел исходный код с множественными возвратами, так как одно большое «если» могло бы стать немного уродливым.

Обратите внимание, что в последнем случае вы просто отступаете от метрики, поскольку логическое ветвление на самом деле одно и то же. Если проверки условий не имеют побочных эффектов (и я действительно надеюсь, что их нет), то я считаю, что высокая цикломатическая сложность является ложной тревогой, поскольку множественные возвраты - это просто более читаемый способ выражения сложной логической логики проверки. 1006 *

Однако это верно только в том случае, если все условия последовательно. Если они распределены по всему коду, и между проверками происходит значимая обработка, тогда метрика указывает на реальную сложность (необходимо проверить больше ветвей). В этом случае вы можете подумать, можно ли логически разбить метод на более мелкие, индивидуально тестируемые части.

...