Where is here?
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:
my ($rank) = grep {$sorted_array[$_] eq $my_target} 0..$#sorted_array;
But even so, I would prefer to see it bundled into a
function like so:
sub index_of {
my $thing = shift;
my @indexes = grep $_[$_] eq $thing, 0..$#_;
wantarray ? @indexes : $indexes[0];
}
and now you can write:
my $rank = index_of($my_target, @sorted_array);
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.
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:
my %sorted_index;
$sorted_index{$sorted_array[$_]} = $_ foreach 0..$#sorted_array;
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 = map {($_, $sorted_array[$_])} 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 @doubled = map {($_, $_)} 1..50_000;
to see what I mean.
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. |