in reply to Do as I say. Not as I do

Hmm. Not sure I agree the second one is clearer than the first. A multi-line map can be hard to follow, and it's not clear to me that "Creating a new array has got to be better than mysteriously transforming one."

To a great extent, understandability is in the eye of the beholder. Nonetheless, I think a couple of very minor changes or additions to your first code would have made your intent clearer, more so than using map to cons up a new array. Here's what I'd suggest:

  1. Add a comment just above the loop saying # Modify input array in-place.Now your intent is very clear.
  2. Name the array element instead of using $_. I personally love and use $_ a lot, but if I have to refer to it explicitly, I stop and ask whether a named variable would be better. In this case, I don't know what's in @in, so I'd have to settle for $elem, $node, or something.
  3. A very minor point, but I really dislike leaning-toothpick regular expressions. I'd change the second split to use delimiters other than /.
Otherwise, I think it's very clear what the first loop is doing. Here's my take on it: 
# Modify the input array in-place
foreach my $elem (@in) {
  my ($count, $file) = split /\s+/, $elem;

  $file =~ s/^$in_prefix//;

  my ($campaign, $month, $fname) = split |/|, $file;

  $elem = {
         campaign => $campaign,
         month => $month,
         file => $fname,
         count => $count,
         sort => "$campaign:$month:$file"
  };
}

[download]

Update:
Changed split to split $elem, not $_. Thanks, runrig! (But let the goof caught by Hofmator stand. ;-)
Fixed split again. This is not my day....

Replies are listed 'Best First'.
Re: Re: Do as I say. Not as I do
by Hofmator (Curate) on Dec 17, 2001 at 22:17 UTC
    I'd change the second split to use delimiters other than /.

    Good point, VSarkiss, but when using a different delimiter you have to write 

    split m|/|;

# instead of
split |/|;

    [download]

    I agree fully with your suggestions and like especially your point about making the comment. This solution was not mentioned elsewhere. A perl programmer should know that $_ (or $elem) is an alias for the actual element. Nevertheless overreading this is easy, so the comment is a good solution.

    -- Hofmator

[reply]
[d/l]
      Good point, VSarkiss, but when using a different delimiter you have to write 
      split m|/|;

# instead of
split |/|;

      [download]
      Or you can say: split "/"; (though " " is, of course, a special case, but for other simple one character (non-metacharacter) splits I like to use quotes :) If you're splitting on literal ".", you can't use "." but you could use "\\." but then I'd just assume use /\./ ...Got all that?
[reply]
[d/l]
[select]

In Section Meditations