Monday, May 26, 2008

listify() and Perl::Critic

Before I start, I just want to make clear to anyone unfamiliar with them, both perltidy and Perl::Critic are pretty comprehensively customizable. But, I am interested in the default settings. The reason is this: on arbitrary choices like indenting by 4 spaces or indenting by 3, if everyone stuck to the defaults, code would be much more uniform across code bases. I guess as I get older, I see much more of the value in conformity. :)

In my last blog entry, I made manual changes to make my code more clear. So, I ran the code through Perl::Critic to gain some additional clarity. This was a full example program and not just the subroutine from the previous blog entry. The final program is below:


use strict;
use warnings;
use version; our $VERSION = qv('0.1');
use Data::Dumper;

my @list = (
'Alabama',        'Alaska',       'Arizona',      'Arkansas',
'California',     'Colorado',     'Connecticut',  'Delaware',
'Florida',        'Georgia',      'Hawaii',       'Idaho',
'Illinois',       'Indiana',      'Iowa',         'Kansas',
'Kentucky',       'Louisiana',    'Maine',        'Maryland',
'Massachusetts',  'Michigan',     'Minnesota',    'Mississippi',
'Missouri',       'Montana',      'Nebraska',     'Nevada',
'New Hampshire',  'New Jersey',   'New Mexico',   'New York',
'North Carolina', 'North Dakota', 'Ohio',         'Oklahoma',
'Oregon',         'Pennsylvania', 'Rhode Island', 'South Carolina',
'South Dakota',   'Tennessee',    'Texas',        'Utah',
'Vermont',        'Virginia',     'Washington',   'West Virginia',
'Wisconsin',      'Wyoming',

listify( \@list, 11 );

print Data::Dumper::Dumper( \@list );

sub listify {
   my ( $aref, $cc ) = @_;
   if ( ref $aref eq 'ARRAY' && $cc > 0 ) {
       my $j;
       for ( my $i = 0 ; $i <= $#$aref ; $i += $cc ) {
           push @$j, [ @$aref[ $i .. $i + $cc - 1 ] ];
       $#{ $j->[ $#{$j} ] } = $#$aref % $cc;
       @$aref = @$j;
       return 1;
Perl::Critic comes with a command-line utility, perlcritic. The default minimum severity level for deviations from the standard is 5, the most severe. Since I was looking for enlightenment, I wanted to see everything it had. I ran perlcritic -severity 1 This is an example of its output:
Code is not tidy at line 1, column 1. See page 33 of PBP.
(Severity: 1)

RCS keywords $Id$ not found at line 1, column 1. See page 441 of
PBP. (Severity: 2)

RCS keywords $Revision$, $HeadURL$, $Date$ not found at line 1,
column 1. See page 441 of PBP. (Severity: 2)

RCS keywords $Revision$, $Source$, $Date$ not found at line 1,
column 1. See page 441 of PBP. (Severity: 2)

No "VERSION" variable found at line 1, column 1. See page 404 of
PBP. (Severity: 2)

Code before warnings are enabled at line 6, column 1. See page
431 of PBP.  (Severity: 4)

Double-sigil dereference at line 35, column 25. See page 228 of
PBP. (Severity: 2)

C-style "for" loop used at line 41, column 9. See page 100 of
PBP. (Severity: 2)
I ignored perlcritic's demands for RCS stuff like $Id$ which I don't need in a throw-away program. Other than that, I addressed all of its complaints and made the following changes:
  • perlcritic complains that code is not "tidy". That's pretty damn cool (though not unexpected as it is in PBP). I ran perltidy -o example.out, compared the differences, then copied example.out over . The prominent changes were:
    • Lines longer than 80 characters were broken up into multiple lines
    • Other whitespace changes were made. It did actually move a line up: where I had ended my if ( with a close-paren semicolon on its own line, it brought that up to the previous line. Not sure I agree with that choice, but I kept it in the name of Zen.
  • It wanted me to use warnings. I have mixed feelings about warnings because I hate things like having to no warnings 'once';, but I can't deny it has helped me before. Typically, I only compile with warnings (perl -Wc as that usually gives me all I need.
  • It wanted me to use version. Even though it's a throw-away, I complied.
  • It did not like my c-style for loop. Mixed feelings again here - this particular for loop is very simple in structure. I re-wrote it as an exercise, though.
  • Final element in array should be -1, not $#array. Doh. $#array is just something I've been in the habit of using. But when you are actually referencing the final element, -1 looks a whole lot nicer.
  • In response to many double-sigil complaints, I created variable $aref_elements to reduce clutter. This was something that I should have discovered during refactoring anyway.
  • Perl::Critic prefers $#{$aref} to $#$aref. That is interesting. My initial feeling is that it looks much busier with the curlies, but I can see Damian's point. Once you are comfortable looking at them, consistent use erases a lot of ambiguity.

No comments: