Re^3: "advanced" Perl functions and maintainability

Nothing, but the way you state this suggests there's something wrong with map and/or grep. Why is that?

I never said that there was something wrong with grep or map, I said that the original poster's usage of grep and map combined--which didn't even work--was bad, and that in general I preferred foreach over greps and maps with multi-statement blocks.

Neither is inherently more clear than the other - people finding one construct more clear than the other usually base this preference (knowingly or not) on their experience with other languages.

I base my preference on the knowledge that variable names are perhaps the single most powerful form of documentation programmers have available to them, and one construct encourages you to give a name to list iterator while the other doesn't. Furthermore, map blocks are indeed meant to be short because each one is evaluated for its result. Thus, if you have a single statement map block like this:

my @content = map {$_->content} @responses;
[download]

It all works out nicely, but if you want to do anything more complicated, you end up combining multiple statements into one, like so:

my @content = map { escape_html(convert_newlines($_->content)) } @resp
+onses;
[download]

Or as Brian showed, separating them and then tacking on the result at the very end to make sure the block gets evaluated properly:

my @content = map {
    my $content = $_->content;

    convert_newlines($content);
    escape_html($content);

    $content;
} @responses;
[download]

And not only that, in my personal aesthetic opinion, grep/map combined with $_ are just plain uglier than foreach is, especially when you need to toss in a "+" or two to help the parser, as the OP did.

What is objectionable is to suggest that you can objectively say that foreach is clearer than map.

Did you actually bother to check the original node? Here, you compare one with the other and you decide which you think is more readable:

my $parts = [ 'Part1', 'Part2','Part3' ];
my $newLoop;

for (@$parts) {
  my $hash = { PARTNAME => $_ };

  $hash->{SELECTED}++
      if $_ eq $row->{title};

  push @$newLoop, $hash;
}

# ----

my $parts = [ 'Part1', 'Part2','Part3' ];
my $newLoop = [ map {{ PARTNAME => $_,
                        $_ eq $row->{title}
                           ? (SELECTED => 1)
                           : ()
                    }} @$parts ];
[download]

Stating that foreach is clearer than map just shows your inexperience in programming techniques.

Well, I've posted lots of code here before, so I'm more than willing to let my technique speak for itself. I'll admit that I often use grep and map freely with no inhabitions at all, but not in code that ever sees the light of day. I would be less than thrilled if I was presented with a large codebase to work on full of constructs like the OP's.

Comment on Re^3: "advanced" Perl functions and maintainability Select or Download Code

Replies are listed 'Best First'.
Re^4: "advanced" Perl functions and maintainability by Anonymous Monk on Dec 13, 2004 at 14:34 UTC
Oh, I would write the map as: `my $newLoop = [ map { my $hash = {PARTNAME => $_}; $hash->{SELECTED} = 1 if $_ eq $row->{title}; $hash; } @$parts ];` [download] Note how remarkably the block looks like the block of the foreach loop. Except that it doesn't have the push on the last line (if variables are so important, why use an out-of-scope variable for its side-effect if you don't have to?), just a result. IMO, clear. The map-block does only what it needs to do: get a value, calculate the result. It doesn't get globbered with array constructing code. I base by preference on the knowledge that variable names are perhaps the single most powerful form of documentation programmers have available to them, and one construct encourages you to give a name to list iterator while the other doesn't. Ah, that explains why your alternative to map starts with: `for (@$parts)` [download] Nice name for the list iterator! Furthermore, map blocks are indeed meant to be short because each one is evaluated for its result. Does that mean that subroutines that have a return value have short blocks, and any subroutine that has a long block isn't supposed to have a return value? Or does this piece of logic only apply to map blocks?	[reply] [d/l] [select]
Re^5: "advanced" Perl functions and maintainability by William G. Davis (Friar) on Dec 13, 2004 at 14:58 UTC
Ah, that explains why your alternative to map starts with: `for (@$parts)` Nice name for the list iterator! It isn't my alternative. Neither of those implementations were mine. Why don't you actually go read the original node? IMO, clear. And I agree, your code is clear, but not as clear as this: `my $newLoop = []; foreach my $part (@$parts) { my %hash; $hash{PARTNAME} = $part; $hash{SELECTED} = 1 if ($part eq $row->{title}); push(@$newLoop, \%hash); }` [download] Longer, yes, but clearer. Code is often read many more times than it is written, and it's for this reason I usually choose the clearer alternative over the shorter one. And before you jump on me, I wouldn't use variable names like $newLoop or $hash or $part in production code, but that's what your example and the other examples used, so I used them as well. Does that mean that subroutines that have a return value have short blocks, and any subroutine that has a long block isn't supposed to have a return value? Or does this piece of logic only apply to map blocks? An implicit return value? In my opinion, no matter how short a subroutine is, if it returns a meaningful value, it should have an explicit return statement indicating as much, that way you can tell just by looking at it whether its return value is to be ignored. (See above.).	[reply] [d/l] [select]
Re^6: "advanced" Perl functions and maintainability by Aristotle (Chancellor) on Dec 20, 2004 at 04:36 UTC
So I guess all that means that you avoid `do { }` blocks as well? After all, you can't put an explicit return in there either. Pity, you're missing out on a great tool to clarify and structure code. You'll find stuff like this all over my code: `my $timestamp = { my( $year, $month, $date ) = ( localtime )[ 5, 4, 3 ]; $year += 1900; $month++; join '-', $year, $month, $date; };` [download] No leaked variables. Also trivial to refactor into a sub if I notice I need it more than once. And if variable names are so important (yes, they are), what's that `$hash` doing there in your code? In any case, `my @newLoop = map +{ PARTNAME => $_, SELECTED => $_ eq $row->{ title }, }, @$parts;` [download] (No, it doesn't cost much extra memory. The base memory overhead of an anonymous hash is so large it easily overshadows an empty key. I'd build this structure inside-out by saying `{ SELECTED => 42, PARTS => [ 'foo', 'bar', 'baz', ... ] }`, which takes a fraction of the memory and also makes updating the selected entry an atomic O(1) vs a two-step O(n) operation, but that's just me.) Makeshifts last the longest.	[reply] [d/l] [select]