in reply to "advanced" Perl functions and maintainability

i was trying to get a combination of map and grep working to set a selected element to pass to HTML::Template without using a foreach loop.

Um, why? What's wrong with foreach loops?

In my opinion, code like this:

my $parts = [ 'Part1', 'Part2','Part3' ]; my $newLoop = [ grep { $_->{SELECTED}++ if $_->{PARTNAME} eq $row->{title}, $_ } map +{ 'PARTNAME' => $_ }, @$parts ] ;

is bad even if it works, which--as you found out--it doesn't. grep/map are great as list filters, when you want to build one list from another, but they force you to stuff all of your code in one tiny block/expression. foreach is the general-purpose list iterator, and you should always be ready to fallback on it if grep or map get too hairy, like revdiablo showed you:

for (@$parts) { my $hash = { PARTNAME => $_ }; $hash->{SELECTED}++ if $_ eq $row->{title}; push @$newLoop, $hash; }

foreach is your friend.

UPDATE: -2? What was so objectionable about suggesting Perl programmers use a clearer construct over a shorter one?

Replies are listed 'Best First'.
Re^2: "advanced" Perl functions and maintainability
by brian_d_foy (Abbot) on Dec 13, 2004 at 02:34 UTC
    grep/map are great as list filters, when you want to build one list from another, but they force you to stuff all of your code in one tiny block/expression.

    Well, this problem is to build one list from another. Your example builds one list from another. To be clear though: grep is a filter, but map is a transformer.

    Neither map nor grep force you to do stuff anything anywhere. There is no magic rule that says it has to fit on one line. You're doing all that work in the foreach anyway (although you're stuffing much more stuff into it).

    Consider what is important in the problem. Is it iterating through each element and doing something, or is it creating a new list? In this case, it sure looks like the new list is the point of the exercise, so it should get top billing. When you use map, you emphasize the new list. When you use foreach, you bury the list creation in a bunch of other code. The new list doesn't stand out. Using foreach in this instance may make the syntax readable to anyone with a week's worth of Perl under their belt, but it doesn't make the program logic more readable.

    @$newLoop = map { my $hash = { PARTNAME => $_ }; $hash->{SELECTED}++ if $_ eq $row->{title}; $hash; } @$parts;
    --
    brian d foy <bdfoy@cpan.org>

      I disagree. For complex expressions, foreach can always be made more readable than map, because foreach lets you explicitly name the variable that stores each element from the list you're iterating over. Again, this is why I prefer it for anything more than single-statement greps or maps. It may take a little more typing, but in the end, it's clearer and more readable, and yeah, I guess a little easier on the novices too...

        For complex expressions, foreach can always be made more readable than map, because foreach lets you explicitly name the variable that stores each element from the list you're iterating over.
        foreach my $foo (@array) {... $foo ...} map {my $foo = $_; ... $foo ...} @array
Re^2: "advanced" Perl functions and maintainability
by Anonymous Monk on Dec 13, 2004 at 10:40 UTC
    Um, why? What's wrong with foreach loops?
    Nothing, but the way you state this suggests there's something wrong with map and/or grep. Why is that?

    IMO, foreach and map/grep are equivalent. 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. Many Perl programmers have learned programming in a language that has for/foreach like loops, but don't have map like constructs (C and Java for instance) - and many, if not all, Perl books and tutorials teach for/foreach long before they introduce map, and spend a lot more time explaining for/foreach than map or grep. People who are familiar with Lisp, Scheme, or other functional programming languages consider map/grep less exotic.

    grep/map are great as list filters, when you want to build one list from another, but they force you to stuff all of your code in one tiny block/expression. foreach is the general-purpose list iterator, and you should always be ready to fallback on it if grep or map get too hairy
    Either you don't understand map/grep, you don't understand for/foreach, or you don't understand either. A foreach construct looks like:
    foreach (LIST) {BLOCK} # or foreach ITERATOR (LIST) {BLOCK}
    while a map construct looks like:
    map {BLOCK} LIST; # or map EXPRESSION, LIST;
    So, either construct requires a keyword, a block and a list. The difference is that with one construct you place the list before the block (and need some addition keystrokes), and the block last. The other construct places the list at the end, with the block proceeding the list. In many cases, the blocks used can be identical - the block used by map can be as large as the one used by foreach (or should I say that foreach forces you to stuff all your code in a tiny block?).
    What was so objectionable about suggesting Perl programmers use a clearer construct over a shorter one?
    Nothing at all. What is objectionable is to suggest that you can objectively say that foreach is clearer than map. Stating that foreach is clearer than map just shows your inexperience in programming techniques.

      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;

      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;

      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;

      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 ];

      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.

        Oh, I would write the map as:
        my $newLoop = [ map { my $hash = {PARTNAME => $_}; $hash->{SELECTED} = 1 if $_ eq $row->{title}; $hash; } @$parts ];
        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)
        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?
Re^2: "advanced" Perl functions and maintainability
by diotalevi (Canon) on Dec 13, 2004 at 00:20 UTC
    I'd only note that using foreach/push instead of a single assignment requires more work on the part of the interpreter. It'd be a trade-off on whether I valued the work savings on the part of the interpreter over the ability of novices to read the code.

      Right, but "$string =~ /$substring/" requires more work on the part of the interpreter than plain "index($string, $substring)" does, yet only a minority of Perl hackers bother to touch index/rindex at all.

        Does it, really? As far as I can tell, the optimizations in the regular expression engine nicely cover the kinds of matches that could be done with index. Try running some benchmarks and see.