byterock

Well looks like old fiend smartypants was up to her old tricks again.

You know the type, spends the day at work in some IRC, never forgets the minutia of page 67 of the help file of a sys-admin command, has memorized and and has a better version every regex ever written, knows she is always right and in the bosses eyes can never make a poor choice when designing code.

Anyway let's get on with the code example. Sometimes the ugly way to do
things is the best. In this case processing a large array where lets say the following has to be extracted,

1. The maximum value
2. The minimum value
3. Clean out any duplicates
4. Group the data into 3 sets

Simple enough really. But then I saw the code (changed to protect the guilty and save space)

use List::MoreUtils qw(minmax uniq part);
my ($set_min, $set_max) = minmax @set;
my @unique_set = uniq @set;
my $not_used;
my ($not_used,$sub_set_a) = part { $_ <= $a } @set;
my ($not_used,$sub_set_b) = part { $_ > $a and $_<=$b } @set;
my ($not_used,$sub_set_c) = part { $_ > $b } @set;

Ok not much here to complain about Yes I agree this does read well and does everything correctly but is it good code?

In most cases I would say yes but in this case the @set happened to have several years worth of data and thus thousands of values.

So would not this be better??

my ($set_min,$set_max,%unique,@sub_set_a,@sub_set_b,@sub_set_c) foreach my $item (@set){ $set_max = $item if ($item > $set_max); $set_min = $item if ($item < $set_min || !$set_min ); $unique{$item}=1; push(@sub_set_a,$item) if ($item <= $a); push(@sub_set_b,$item) if (($item > $a) and ($item <= $b)); push(@sub_set_c,$item) if ($item > $b); }

From max of 5x (assuming maxmin does only 1 iteration) down to 1x. I don't care the
the List::MoreUtils using 'C' with the 'XS' you still have the possibility of a
huge amount of extra iteration. It doesn't take a higher maths degree to argue that one.

The point being that Smartypants wanted show off by using some nifty-cool module whiteout actually looking at problem.

Don't get me wrong the List::MoreUtils are great when you have one thing to do a to a list.

Have more than one task? Take a few seconds and think a bout it.