in reply to grep instead of foreach?
Scratch that.
Wherever "here" is, that advice is bad. If you are not using the return value, do not use grep or map! Why not? Well you have made the code conceptually more complex, you are forcing Perl to store more information so it wastes memory, you are forcing Perl to do more work so it runs more slowly, and for what? Just to look cool???
If you will use the return value sensibly, then by all means use map or grep - that is why they exist. But don't use them to replace loops that could be more clearly written as loops!
In this case a grep actually is pretty natural:
But even so, I would prefer to see it bundled into a function like so:my ($rank) = grep {$sorted_array[$_] eq $my_target} 0..$#sorted_array;
and now you can write:sub index_of { my $thing = shift; my @indexes = grep $_[$_] eq $thing, 0..$#_; wantarray ? @indexes : $indexes[0]; }
which is already 2 wins. First of all you lose having to worry about scalar vs list context (I get that right in the function). Secondly rather than looking at a complex construct and saying, "What does that do?" you can look at a function name that does a decent job of documenting it. And if you wanted to make it more efficient by only searching until you find your (hopefully unique) element, well you can.my $rank = index_of($my_target, @sorted_array);
But if you were going to need more than a half-dozen or so ranks, you are probably better off producing a reverse hash lookup like so:
Now I can just see your co-workers snort. Why doesn't he use a map? After all there is a perfectly good map out there where you use the return value:my %sorted_index; $sorted_index{$sorted_array[$_]} = $_ foreach 0..$#sorted_array;
And of course there is a perfectly good reason not to do this. And it is because I don't know whether you are using 5.6.1. If you are then the map will work very nicely. If you are not then a map that produces 2 for 1 like this is very slow. Try:my %sorted_index = map {($_, $sorted_array[$_])} 0..$#sorted_array;
to see what I mean.my @doubled = map {($_, $_)} 1..50_000;
Secondly I wanted to point out the inline loop construct that allows them to feel like cool Perl hackers without the bad grep/map constructs. But the cool Perl hackers tend to solve that problem something like this:
my %sorted_index; @sorted_index{@sorted_array} = 0..$#sorted_array;
UPDATE
Albannach caught an accidental pair of opening code
tags rather than closing it. Fixed.
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re: Re (tilly) 1: grep instead of foreach?
by iakobski (Pilgrim) on May 22, 2001 at 14:21 UTC | |
by bjelli (Pilgrim) on May 22, 2001 at 14:41 UTC |