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

C/C++ several "case:" labels together treated as separated #82

Closed vicgonzalez closed 7 years ago

vicgonzalez commented 8 years ago

If several "case:" labels in a switch are together (without other statements or breaks in the middle) they are treated as separate paths increasing the complexity number. switch(var){ case 1: case 2: doSomething(); break; } Since they are like ORs, they should be treated as one single execution path. Has to be taken into account also for the modified cyclomatic complexity.

terryyin commented 8 years ago

@vicgonzalez theoretically, each "case" is just an if statement and adding one branch, leaving the branch empty might reduce the possible output but doesn't reduce the possible inputs. From testing perspective, the amount of tests needed is decided by the possible inputs not the outputs. Just like an if statement with one condition will add 1 cc with or without an else statement. And by the way, if an 'if' statement's condition contains an "OR" or "AND" it will be counted as 2. Again, think of the amount of possible input instead of output.

However, in reality the conditions in a switch/case statement usually have some inherent constraints and do not really add that much possible inputs (that are meaningful). So there're different opinions.

Some people contributed a -m option to Lizard to "Calculate modified cyclomatic complexity number," which will count the entire switch/case as one CC. So just run:

   lizard -m

Will do.

Personally, I would not use it. Because if there is really an inherent constraint with the cases, I would use an array or a dictionary to ensure that constraint, instead of using switch/cases and leave too many possibilities. Or polymorphism, as many OO people would suggest, which in my speculation, might be a better alternative for several "cases" sharing one piece of code.

vicgonzalez commented 8 years ago

I've done a little research and I think I'm not the first with that kind of issue: https://github.com/jshint/jshint/issues/840 https://github.com/jshint/jshint/pull/2611 Some mention that in pages 26, 27 of http://www.mccabe.com/pdf/mccabe-nist235r.pdf states that fall through cases count as 1... May be the issue is polemic enough to include a flag to choose either way according to one's taste...

terryyin commented 8 years ago

Hmm, are you suggesting that I should make the modified CCN as default? I’m thinking if I should conduct a survey...

On 18 Jan 2016, at 8:58 PM, vicgonzalez notifications@github.com wrote:

I've done a little research and I think I'm not the first with that kind of issue: jshint/jshint#840 https://github.com/jshint/jshint/issues/840 jshint/jshint#2611 https://github.com/jshint/jshint/pull/2611 Some mention that in pages 26, 27 of http://www.mccabe.com/pdf/mccabe-nist235r.pdf http://www.mccabe.com/pdf/mccabe-nist235r.pdf states that fall through cases count as 1... May be the issue is polemic enough to include a flag to choose either way according to one's taste...

— Reply to this email directly or view it on GitHub https://github.com/terryyin/lizard/issues/82#issuecomment-172509165.

vicgonzalez commented 8 years ago

No, what I meant is that for 'not modified' cyclomatic complexity there are people that counts each case separately (like you) and people that counts fall through cases as one, like McCabe in: http://www.mccabe.com/pdf/mccabe-nist235r.pdf of people in the similar discussion threads jshint/jshint#840 jshint/jshint#2611 My suggestion is to add a new flag (apart from the modified -m) to calculate the 'not modified' CC under that point of view. Some tools do it "your" way and some other tools (like QAC from programming research) do it "the other" way, but it would be a great value a tool where you can choose which way you prefer.

terryyin commented 8 years ago

@vicgonzalez I've made a new option "-EMcCabe" to enable the McCabe version of cyclomatic complexity. Here's the implementation: https://github.com/terryyin/lizard/blob/master/lizard_ext/lizardmccabe.py

I've created a new release (1.10.2) with some other fixes. Thank you so much for the idea.