Swiftlint: Cyclomatic Complexity is improperly measured for case statements

Created on 1 Feb 2016  路  7Comments  路  Source: realm/SwiftLint

The following function should have a cyclomatic complexity value of 11, but it's reported as 13.

class func colorFromCode(code: String?) -> UIColor {
    switch code! {
    case "000": return Cache.couplesColor000
    case "001": return // ...
    case "002": return // ...
    case "003": return // ...
    case "004": return // ...
    case "005": return // ...
    case "006": return // ...
    case "007": return // ...
    case "008": return // ...
    case "009": return // ...
    default: return UIColor.clearColor()
    }
}

This reports the following violation:

<nopath>:1:7: warning: Cyclomatic Complexity Violation: Function should have complexity 10 or less: currently complexity equals 13 (cyclomatic_complexity)
bug

Most helpful comment

cyclomatic_complexity:
warning: 25

I changed warning value for the cyclomatic_complexity in .yml file. Warnings gone.

All 7 comments

@jpsim should be easy to fix:

  • switch probably should not be counted. (leaves us with 12)
  • in references I've read cyclomatic complexity of any function starts with 1 (in sample code we can assume that there might be a condition when code in switch won't be executed, which leaves us with one code path).

Will look into that

From: http://gmetrics.sourceforge.net/gmetrics-CyclomaticComplexityMetric.html
Start with a initial (default) value of one (1)

From http://www.mccabe.com/pdf/mccabe-nist235r.pdf:

An implicit default or fall-through branch, if specified by the language, must be taken into account when calculating complexity

Edit
Swift has explicit fall-through, so it is pretty easy to take them into account

Start with a initial (default) value of one (1)

I think you might be misinterpreting this. Yes, CC starts with 1 because you can't have CC of zero. Code that can take zero paths is code that never executes. It's not that you must add one to the number of paths code can take to get its cyclomatic complexity value.

@jpsim agreed, I will remove +1 from the rule.

Done in #463 thanks to @garnett

cyclomatic_complexity:
warning: 25

I changed warning value for the cyclomatic_complexity in .yml file. Warnings gone.

I don't think Cyclomatic Complexity is improperly measured, but in some cases, it doesn't make sense to restrict your code under complexity 10. For example, if your business logic asks you to handle 20 errors differently, it is a good practice to use exhaustive switch to make it incompatible to future changes, and even you write 20 handlers for these errors, you will still have higher complexity.

Of course, if it's a developer who design the business logic, perhaps he will break those 20 errors down to sub categories. But in real life, we don't really have this luxury. For example, if these errors are from backend, frond-end architect may not have the best knowledge to break them down.

In this case, either have a meeting and drag all the architect and business owners and spend hours there, or simply update rules or ignore this rule temporarily. You can't have to enforce all the rules in all places, as long as you have a good reason.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jcarroll-mediafly picture jcarroll-mediafly  路  3Comments

SolaWing picture SolaWing  路  3Comments

castus picture castus  路  3Comments

Tableau-David-Potter picture Tableau-David-Potter  路  3Comments

larslockefeer picture larslockefeer  路  3Comments