While I'm at it I should mention that you have a lot of unnecessary checks in your comparison. You're writing--- External.pm.bak 2008-07-11 22:30:52.000000000 -0700 +++ External.pm 2008-07-11 22:52:14.000000000 -0700 @@ -322,7 +322,7 @@ $comparecode = sub { my $test = shift; return 0 if $test eq $cutoff; - my $winner = ( sort $sortsub( $cutoff, $test ) )[0]; + my $winner = ( sort $sortsub( $test, $cutoff ) )[0]; return $winner eq $cutoff ? 1 : -1; }; }
but if $hours1 and $hours2 are different then you'll never get to the minutes comparison. So you can get rid of those and just have:# .... return -1 if $hours1 < $hours2; return 1 if $hours1 > $hours2; return -1 if $hours1 == $hours2 && $minutes1 < $minutes2; return 1 if $hours1 == $hours2 && $minutes1 > $minutes2; return -1 if $hours1 == $hours2 && $minutes1 == $minutes2 && $sec +onds1 < $seconds2; return 1 if $hours1 == $hours2 && $minutes1 == $minutes2 && $seco +nds1 > $seconds2;
But you can write this more idiomatically using the fact that || short circuits:# ... return -1 if $hours1 < $hours2; return 1 if $hours1 > $hours2; return -1 if $minutes1 < $minutes2; return 1 if $minutes1 > $minutes2; return -1 if $seconds1 < $seconds2; return 1 if $seconds1 > $seconds2;
Incidentally in one of the few useful uses of prototypes in Perl, if a sort sub has a prototype of ($$) then the arguments are passed in as @_ and not $a, $b. So if you writereturn $hours1 <=> $hours2 || $minutes1 <=> $minutes2 || $seconds1 <=> $seconds2 || $Sort::External::a cmp $Sort::External::b;
then you don't have to worry about what package the sort is coming from.my $sortscheme = sub ($$) { my @fieldsa = split(" ", shift); my @fieldsb = split(" ", shift); # ... };
Oh and a final tip? You should use strict.pm. Always.
In reply to Re: Problem with Sort::External 0.16
by tilly
in thread Problem with Sort::External 0.16
by Anonymous Monk
For: | Use: | ||
& | & | ||
< | < | ||
> | > | ||
[ | [ | ||
] | ] |