terryyin / lizard

A simple code complexity analyser without caring about the C/C++ header files or Java imports, supports most of the popular languages.
Other
1.85k stars 250 forks source link

Ignoring default condition in switch clauses #224

Closed d-stahl-ericsson closed 6 years ago

d-stahl-ericsson commented 6 years ago

I recently stumbled upon the fact that Lizard appears to ignore default conditions in switch clauses. To my understanding, these should be counted in McCabe's cyclomatic complexity?

To exemplify, the following method should yield CCN=4, but Lizard reports CCN=3 (example taken from https://blog.sonarsource.com/cognitive-complexity-because-testability-understandability):

String getWords(int number) {   // +1
    switch (number) {
      case 1:                   // +1
        return "one";
      case 2:                   // +1
        return "a couple";
      default:                  // +1
        return "lots";
    }
  }        // Cyclomatic Complexity 4          

I'm only scratching the surface of the Lizard code base, so I have no idea if what I'm doing as unintended side effects, but it seems that changing the CodeReader class does the trick:

    conditions = set(['if', 'for', 'while', '&&', '||', '?', 'catch',
                      'case'])

changed to

    conditions = set(['if', 'for', 'while', '&&', '||', '?', 'catch',
                      'case', 'default'])

Please let me know what you think about this.

terryyin commented 6 years ago

Question: if the only thing inside a switch block is a default (there’s no case), will it add branches to the code?

No, right? Same reason for ‘else’, the complexity for ‘else’ and ‘default’ is already included in the parent structure it belongs to.

On 7 Mar 2018, at 4:57 PM, Daniel Ståhl notifications@github.com wrote:

I recently stumbled upon the fact that Lizard appears to ignore default conditions in switch clauses. To my understanding, these should be counted in McCabe's cyclomatic complexity?

To exemplify, the following method should yield CCN=4, but Lizard reports CCN=3 (example taken from https://blog.sonarsource.com/cognitive-complexity-because-testability-understandability https://blog.sonarsource.com/cognitive-complexity-because-testability-understandability):

String getWords(int number) { // +1 switch (number) { case 1: // +1 return "one"; case 2: // +1 return "a couple"; default: // +1 return "lots"; } } // Cyclomatic Complexity 4
I'm only scratching the surface of the Lizard code base, so I have no idea if what I'm doing as unintended side effects, but it seems that changing the CodeReader class does the trick:

conditions = set(['if', 'for', 'while', '&&', '||', '?', 'catch',
                  'case'])

changed to

conditions = set(['if', 'for', 'while', '&&', '||', '?', 'catch',
                  'case', 'default'])

Please let me know what you think about this.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/terryyin/lizard/issues/224, or mute the thread https://github.com/notifications/unsubscribe-auth/AAwJYsHTbeHy0sxb5gSHwhq78dmK3EfJks5tb6DlgaJpZM4SgE19.

d-stahl-ericsson commented 6 years ago

I have to say that's a pretty convincing argument :) Clearly there seems to be some diverging interpretations here, though, as I can easily find examples both counting and not counting default with a bit of googling. You have a much better understanding of this field than I do - are people simply as confused as I am/was on this point, or what's going on?

terryyin commented 6 years ago

I think it’s pretty common people get confused.

What default and else added to the complexity is the length of the trunk.

On 7 Mar 2018, at 5:17 PM, Daniel Ståhl notifications@github.com wrote:

I have to say that's a pretty convincing argument :) Clearly there seems to be some diverging interpretations here, though, as I can easily find examples both counting and not counting default with a bit of googling. You have a much better understanding of this field than I do - are people simply as confused as I am/was on this point, or what's going on?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/terryyin/lizard/issues/224#issuecomment-371074659, or mute the thread https://github.com/notifications/unsubscribe-auth/AAwJYn3txeVAGsJ_qFXRy3TXYWVtdAI6ks5tb6W_gaJpZM4SgE19.