in reply to Re^3: "advanced" Perl functions and maintainability
in thread "advanced" Perl functions and maintainability

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?

Replies are listed 'Best First'.
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]

      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]

In Section Meditations