Операторы раннего возврата и цикломатическая сложность

Я предпочитаю этот стиль письма с ранней отдачей:

public static Type classify(int a, int b, int c) {
    if (!isTriangle(a, b, c)) {
        return Type.INVALID;
    }
    if (a == b && b == c) {
        return Type.EQUILATERAL;
    }
    if (b == c || a == b || c == a) {
        return Type.ISOSCELES;
    }
    return Type.SCALENE;
}

К сожалению, каждый оператор return увеличивает показатель цикломатической сложности, вычисляемый Sonar. Рассмотрим эту альтернативу:

public static Type classify(int a, int b, int c) {
    final Type result;
    if (!isTriangle(a, b, c)) {
        result = Type.INVALID;
    } else if (a == b && b == c) {
        result = Type.EQUILATERAL;
    } else if (b == c || a == b || c == a) {
        result = Type.ISOSCELES;
    } else {
        result = Type.SCALENE;
    }
    return result;
}

Цикломатическая сложность этого последнего подхода, о котором сообщает Sonar, ниже, чем у первого, на 3. Мне сказали, что это может быть результатом неправильной реализации метрик CC. Или Сонар прав, и это действительно лучше? Эти связанные вопросы, кажется, не согласны с этим:

https://softwareengineering.stackexchange.com/questions/118703/where-did-the-notion-of-one-return-only-come-from

https://softwareengineering.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement

Если я добавлю поддержку еще нескольких типов треугольников, операторы return в сумме существенно изменят метрику и вызовут нарушение Sonar. Я не хочу ставить этому методу // NOSONAR, так как это может замаскировать другие проблемы новыми функциями/ошибками, добавленными к методу в будущем. Поэтому я использую вторую версию, хотя она мне не очень нравится. Есть ли лучший способ справиться с ситуацией?


person janos    schedule 15.09.2014    source источник
comment
Согласно en.wikipedia.org/wiki/Cyclomatic_complexity, CC — это число линейно независимых пути через функцию, которая в обоих случаях 4. Сонар говорит вам что-то другое?   -  person Doc Brown    schedule 15.09.2014
comment
Да. Sonar добавляет +1 к каждому оператору return. Это правило squid:MethodCyclomaticComplexity: dev.eclipse.org/sonar/rules/show/ Более раннее правило (но теперь устаревшее в пользу squid) не имело этого ограничения: dev.eclipse.org/sonar/rules/show/   -  person janos    schedule 15.09.2014
comment
так что этот вопрос на самом деле означает, как предотвратить неправильный расчет CC сонаром, что не подходит для этого сайта. Такие вопросы лучше размещать на stackoverflow. Я отмечаю этот вопрос для миграции.   -  person Doc Brown    schedule 15.09.2014
comment
Вы делаете две вещи здесь. У вас есть условная логика, но также и защитное предложение. Я склонен уважать концепцию единой ответственности, потому что в целом она делает код более читабельным, за исключением защитных предложений в начале функции.   -  person Pieter B    schedule 15.09.2014
comment
Ранее я задавал аналогичный вопрос (stackoverflow.com/questions/23381265/), и оказалось, что сложность, о которой сообщает SonarQube, смешивает цикломатическую сложность и основную сложность.   -  person    schedule 15.09.2014


Ответы (3)


Не совсем ответ, но слишком длинный для комментария.

Это правило SONAR кажется полностью нарушенным. Вы могли бы переписать

b == c || a == b || c == a

as

b == c | a == b | c == a

и получить два очка в этой странной игре (и, может быть, даже некоторую скорость, поскольку ветвление дорого; но это в любом случае на усмотрение JITc).

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

Есть ли лучший способ справиться с ситуацией?

На самом деле у меня есть ответ: для каждого досрочного возврата используйте | вместо || один раз. :D

Теперь серьезно: существует ошибка, запрашивающая аннотации, позволяющая отключить одно правило, которое отмечен как фиксированный. Я не смотрю дальше.

person maaartinus    schedule 15.09.2014

Ваш вопрос касается https://jira.codehaus.org/browse/SONAR-4857. . В настоящее время все анализаторы SonarQube смешивают цикломатическую сложность и существенную сложность. С теоретической точки зрения оператор return не должен увеличивать число копий, и это изменение произойдет в экосистеме SQ.

person Freddy - SonarSource Team    schedule 16.09.2014
comment
Спасибо! Есть идеи когда? - person janos; 17.09.2014
comment
К концу года точно, но пока не могу сказать точнее. - person Freddy - SonarSource Team; 17.09.2014

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

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

(Я использую Sonar и столкнулся с такой же проблемой.)

person segfaultreloaded    schedule 06.11.2014
comment
Я категорически не согласен. Вы правы в том, что в маленьком методе ранние возвраты в порядке. Собственно в маленьком методе примерно все нормально. В большом методе все проблема, и в этом суть. Почему кто-то пишет методы более 30 строк??? Глядя на код, который я написал вчера в полусне, я вижу там некоторую неразбериху, и беспорядок — это большие методы (около 30 строк). Избавьтесь от больших методов, и все будет в порядке. Если код занимает более одного экрана, выполните рефакторинг. - person maaartinus; 12.11.2014