New Perl::Critic policies on the way

ChrisDolan on 2007-07-08T01:40:04

I took a 6-month hiatus from Perl::Critic work (a new baby and a new job among the causes). Now thanks largely to a Perl Foundation grant I'm back writing policies. I greatly appreciate the forthcoming grant dollars, but even more important to me is the external pressure to get something done. It's easy to slack on work that lacks an invested client.

Tonight I wrote the first of the twenty policies funded by that grant (all of them inspired by Perl Best Practices). Tonight's was Perl::Critic::Policy::BuiltinFunctions::ProhibitComplexMapping which insists that the block argument to map() has at most one statement.

My work in happening in the Perl::Critic 1.xxx SVN branch. You can track it at http://perlcritic.tigris.org/svn/perlcritic/branches/Perl-Critic-1.xxx. Read more about the Tigris SVN, which I always find a little non-intuitive.

---

prohibitComplexMappings

mr_bean on 2007-07-08T02:23:54

Multiline mappings is something I do a lot. The Perl::Critic advice is:

"The best solutions to a complex mapping are: 1) write a subroutine that performs the manipulation and call that from map; 2) rewrite the map as a for loop."

I guess the advantage of a for loop over a map is that the array over which you are iterating is upfront.

I always have a problem with the indentation of my multiline maps anyway. How should the brackets around the block line up? How should I line up the final array?

Re:prohibitComplexMappings

ChrisDolan on 2007-07-08T02:48:45

As usual in Perl:Critic, this policy is optional. I flagged it as a "medium" severity (a 3 on a 1 to 5 scale). Multi-statement maps do legitimate uses, but generally should not be used when you can find a better way to say the same thing (which is most of the time). You can always annotate your code such that Perl::Critic will ignore the violation:

  my @out = map {  ##no critic(ProhibitComplexMapping)
      foo($_);
      bar($_);
  } @list;

Note that the "no critic" should be on the line with the word "map", which is the site of the violation.

Re:prohibitComplexMappings

jjore on 2007-07-09T00:21:14

Oooh, argh! Your choice of indentation is painful! If the large chunks are the target, source, map, and map contents, it is semi-sensible to indent based on that. Often, I'd assume this would mean a line per chunk or inlining two or three if they were really simple.

The thing that gets me is you close your map guts and then cuddle the source into that. It's what's driving the entire loop! I'd think it rates pretty highly in importance and should be visually distinct. How about another line for it?

tgt =
    map {
        ...
        ...
    }
    source;

Further, having that giant blob of anti-lint declaration is weird. It's larger and chunkier than all the important stuff to the left.

Re:prohibitComplexMappings

ChrisDolan on 2007-07-09T01:22:47

Yes, well, multi-statement map() is painful. :-) I used that indentation just to emphasize where the "##no critic" goes, if someone were to want it. In real code, I try to make my maps fit readably on one line, or else I choose not to use map.