|Subject:||McCabe calcuation is too high (in PerlCritic)|
|Date:||Wed, 21 Oct 2009 21:02:28 -0500|
|To:||bug-Perl-Critic [...] rt.cpan.org|
|From:||Christopher Dunn <Christopher.Dunn [...] amd.com>|
Cyclomatic complexity should not count the 'else' portion of the flow. elsif yes, but not else. That is probably not obvious from the description, e.g.
The example is pretty clear:
Show quoted text
The complexity is not the number of paths (1 x 2 x 2 == 4). Instead, it is the number decision points (plus 1 for deciding to start the program). An else block can be thought of as code that would have executed anyway if the if-true block were not present, i.e. the default flow of the program. Else-if, however, represents another decision. while/for/etc. are decisions too, which is obvious if you replace them with only ifs and gotos. Because of short-circuiting, booleans have to be counted as well. (That's not true in all languages.) Whether to count just 'switch' or all 'cases' is hotly debated, but to me, a switch is a single decision and evidence of good coding.
if( c1() ) f1(); else f2(); if( c2() ) f3(); else f4();
If someone writes a short function with 5 if-blocks, his complexity is 6 (a good score) not 11 (considered high). People will ignore this altogether if the scores are excessive.
Here is a clearer description:
The fix is easy.
33 Readonly::Hash my %LOGIC_KEYWORDS =>
34 hashify( qw( if else elsif unless until while for foreach ) );
34 hashify( qw( if elsif unless until while for foreach switch ) );
This is a very useful measure, so making it a bit more accurate would be a worthwhile improvement.