| 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> |
http://search.cpan.org/~elliotjs/Perl-Critic-1.080/lib/Perl/Critic/Policy/Subroutines/ProhibitExcessComplexity.pm
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.
http://en.wikipedia.org/wiki/Cyclomatic_complexity#Implications_for_Software_Testing
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 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:
http://www.aivosto.com/project/help/pm-complexity.html
The fix is easy.
33 Readonly::Hash my %LOGIC_KEYWORDS =>
34 hashify( qw( if else elsif unless until while for foreach ) );
should be
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.
~Chris
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.
http://en.wikipedia.org/wiki/Cyclomatic_complexity#Implications_for_Software_Testing
The example is pretty clear:
Show quoted text
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:
http://www.aivosto.com/project/help/pm-complexity.html
The fix is easy.
33 Readonly::Hash my %LOGIC_KEYWORDS =>
34 hashify( qw( if else elsif unless until while for foreach ) );
should be
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.
~Chris