Re: New Quiz: "What does this code do?" (1-December-2006)

Shlomi Fish <shlomif-ik1l9ssToec+JF/[email protected]> Wed, 20 Dec 2006 14:18:38 +0200
Newsgroups gmane.comp.lang.perl.qotw.discuss
Message-ID <[email protected]>
Hi!

On Thursday 14 December 2006 00:05, Garrett, Philip (MAN-Corporate) wrote:
> Owen wrote:
> > On Fri, 1 Dec 2006 16:35:44 +0200
> >
> > Shlomi Fish <shlomif-ik1l9ssToec+JF/[email protected]> wrote:
> >1:   my (@list) =
> >2:   (
> >3:       grep { $cgi->param("$prefix$_") }
> >4:       map { /^${prefix}(\d+)$/ ? ($1) : () }
> >5:       $cgi->param()
> >6:   );
>
> Whenever I see construct like this that confuses me, I rewrite it so
> that the
> code actually goes in the same order as the operations, and turn all the
> temporary lists into named arrays.  If the resulting code is clearer and
> does
> not negatively affect performance in a substantial way, I'll leave it.
>
>     my $prefix = "form_field_";
>
>     # line 5
>     my @param_names = $cgi->param;
>
>     # line 4
>     my @received_fields;
>     for my $param_name (@param_names) {
>         if ($param_name =~ /^${prefix}(\d+)$/) {
>             my $field_number = $1;
>             push @received_fields, $field_number;
>         }
>     }
>
>     # line 3
>     my @list;
>     for my $field_number (@received_fields) {
>         if ($cgi->param("${prefix}${field_number}")) {
>             push @list, $field_number;
>         }
>     }
>
>     # @list now contains a list of numbers for which there are
>     # populated form fields named "form_field_<number>".
>     # ** it might contain duplicates, too
>     # ** "populated" means non-empty and non-zero
>

Wow-wa! You took a 3-liner and turned it into a 10-15 lines verbose 
monstrosity. If you're experienced enough you'll find the more concise map 
and grep version, as easy to understand as your code if not more. (Because 
there's less to read). See:

http://www.paulgraham.com/popular.html (search for "brevity")
http://www.paulgraham.com/power.html

> Another way, but it doesn't really clear things up much:
>
>     my @params  = $cgi->param;
>     my @numbers = map { /^${prefix}(\d+)$/ ? ($1) : () } @params;
>     my @list    = grep { $cgi->param("$prefix$_") } @numbers;
>

If you want the order of operations in the code to match their execution 
order, you may want to look at the Pipe module:

http://cpan.uwinnipeg.ca/dist/Pipe

Using it one can write this untested code:

my @list = Pipe->for($cgi->param)
  ->map(sub { shift =~ /^${prefix}(\d+)$/ ? ($1) : ()})
  ->grep( sub { $cgi->param("$prefix$_[0]") })->run;

Regards,

	Shlomi Fish

---------------------------------------------------------------------
Shlomi Fish      shlomif-ik1l9ssToec+JF/[email protected]
Homepage:        http://www.shlomifish.org/

Chuck Norris wrote a complete Perl 6 implementation in a day but then
destroyed all evidence with his bare hands, so no one will know his secrets.