Herkum has asked for the wisdom of the Perl Monks concerning the following question:

I have this sort routine,
for my $team ( sort {$b->get_wins_total <=> $a->get_wins_total || $b->get_wins_division <=> $a->get_wins_division || $b->get_wins_home <=> $a->get_wins_home || $b->get_runs_scored <=> $a->get_runs_scored || $a->get_runs_allowed <=> $b->get_runs_allowed || $a->get_reg_schedule_id <=> $b->get_reg_schedule_id } @{$self->teams} ) { }

I would like to reduce the sort parameters to a function call, how would I make it like this,

for my $team ( sort standings() @{$self->teams}) { }

Suggestions anyone?

Replies are listed 'Best First'.
Re: Refactor my sort
by jettero (Monsignor) on Dec 22, 2006 at 16:02 UTC

    Well, $a and $b are globals... so just put your conditionals into a sub and then omit your parens in the sort-call and it'll work pretty much just like you have it. There are examples of that on the sort page. You'll be amused to see how close you really were.

    -Paul

      Looking at the sort documentation, you are totally on the mark, THANKS! :)
Re: Refactor my sort
by imp (Priest) on Dec 22, 2006 at 16:07 UTC
      Looking at it, you are probably right that my code would be a good candidate for this. However I am reading the documentation, and I will be honest, I don't understand it.
        Herkum:

        My take on it is this: the comparison operator on a string is a bit slow since it has to take locale and such into account. The GRT basically packs the data into a string of bytes such that a simple numeric comparison would reliably sort the data.

        This saves you time because rather than go through the locale-conversion code to order the strings each time they're accessed, you perform the conversion once (during pack), then you just treat the keys as integers X bytes long, which are much simpler to compare.

        --roboticus

      I'm not so sure. Functions calls are expensive, but are they expensive enough and is there enough data to sort to make the drop in readability worth the gain in speed?

      Here's something you can use to benchmark.

      our @unsorted; local *unsorted = $self->teams(); my @sorted; # Allocate memory up front. $#sorted = $#unsorted; # Is it possible to do this faster? for my $i (0..$#sorted) { local *_ = \($unsorted[$i]); $sorted[$i] = pack 'C6N', 255-($_->get_wins_total()), # Descending 255-($_->get_wins_division()), # Descending 255-($_->get_wins_home()), # Descending 255-($_->get_runs_scored()), # Descending $_->get_runs_allowed(), # Ascending $_->get_reg_schedule_id(), # Ascending $i; } # In-place lexical sort. @sorted = sort @sorted; for my $i (0..$#sorted) { $sorted[$i] = $unsorted[unpack('x6N', $sorted[$i])]; }
      Q: And what sort of an honest man are you?
      A: I'm a sort-of honest man.
      This is indeed a good candidate for Guttman Rosler. But note carefully. He wants to sort some keys ascending, others descending. In addition to the pack, he'll need a transform which will turn descending keys into ascending ones (or vice versa.) E.g. subtract each descending key from 1000. That will only add trivially to the computation time. But it will add another layer of difficulty for the maintainer's comprehension.

      throop
      But many that are first shall be last; and the last shall be first. — Matt 19:30
Re: Refactor my sort
by eyepopslikeamosquito (Archbishop) on Dec 22, 2006 at 19:57 UTC

    One thing you should be aware of is that sort subs are currently not threadsafe, due to difficult-to-fix perl bug #30333, described in Perl threads sort test program crashes. That is, the test program given in that node still crashes with perl 5.8.8.

    It's the lightweight mechanism used to call sort subs that is not thread-safe ... and fixing it, without harming the performance of all sub calls, has so far proved problematic. I doubt this bug will be fixed anytime soon, so if you need your code to be threadsafe, you should avoid sort subs and stick to using a sort block.

Re: Refactor my sort
by Util (Priest) on Dec 23, 2006 at 05:03 UTC

    Your question has already been well-answered. I am posting code for 5 methods, for variety and comparison.

    1. Your original code, placed in a sort sub (++jettero)-- Simple!
    2. Schwartzian Transform -- Fast!
    3. Guttman-Rosler Transform -- Faster!
    4. Sort::Maker -- Concise!
    5. Sort::Key -- Even more concise!

    The simple sort sub is probably best for your needs. Fast enough, concise enough, and requiring no extra modules. The two transforms are included just as an illustration of how to write them yourself; they are more verbose than the simple sub, and you would only need their speed if you had many teams, or if your method calls were slow. Sort::Key and Sort::Maker are not core, but are almost as fast as the hand-coded transforms, and allow for very maintainable code. Sort::Maker is my favorite.

    use strict; use warnings; # Heavily abused fake object class package Teams; sub get_wins_total { $_[0]->{ WT } } sub get_wins_division { $_[0]->{ WD } } sub get_wins_home { $_[0]->{ WH } } sub get_runs_scored { $_[0]->{ RS } } sub get_runs_allowed { $_[0]->{ RA } } sub get_reg_schedule_id { $_[0]->{ ID } } sub print_me { print map(sprintf("%7d\t", $_[0]->{$_}), qw( WT WD WH RS RA ID )), + "\n"; } package main; # Herkum's sort sub sub standings { $b->get_wins_total <=> $a->get_wins_total or $b->get_wins_division <=> $a->get_wins_division or $b->get_wins_home <=> $a->get_wins_home or $b->get_runs_scored <=> $a->get_runs_scored or $a->get_runs_allowed <=> $b->get_runs_allowed or $a->get_reg_schedule_id <=> $b->get_reg_schedule_id } # Schwartzian Transform sub sort_teams_by_standing_ST { return map { $_->[0] } sort { $b->[1] <=> $a->[1] or $b->[2] <=> $a->[2] or $b->[3] <=> $a->[3] or $b->[4] <=> $a->[4] or $a->[5] <=> $b->[5] or $a->[6] <=> $b->[6] } map {[ $_, $_->get_wins_total, $_->get_wins_division, $_->get_wins_home, $_->get_runs_scored, $_->get_runs_allowed, $_->get_reg_schedule_id, ]} @_; } # Guttman-Rosler Transform sub sort_teams_by_standing_GRT { return @_[ map { unpack 'x[l6] l', $_ } sort map { pack 'l7', -$_[$_]->get_wins_total, -$_[$_]->get_wins_division, -$_[$_]->get_wins_home, -$_[$_]->get_runs_scored, $_[$_]->get_runs_allowed, $_[$_]->get_reg_schedule_id, $_, } 0..$#_ ]; } # CPAN module Sort::Maker use Sort::Maker; sub sort_teams_by_standing_SM; make_sorter( name => 'sort_teams_by_standing_SM', qw( GRT ), number => [ qw( descending unsigned ), code => sub { $_->get_wins_ +total } ], number => [ qw( descending unsigned ), code => sub { $_->get_wins_ +division } ], number => [ qw( descending unsigned ), code => sub { $_->get_wins_ +home } ], number => [ qw( descending unsigned ), code => sub { $_->get_runs_ +scored } ], number => [ qw( ascending unsigned ), code => sub { $_->get_runs_ +allowed } ], number => [ qw( ascending unsigned ), code => sub { $_->get_reg_s +chedule_id } ], ) or die "make_sorter: $@"; # CPAN module Sort::Key use Sort::Key::Maker sort_teams_by_standing_SKM => sub { $_->get_wins_total, $_->get_wins_division, $_->get_wins_home, $_->get_runs_scored, $_->get_runs_allowed, $_->get_reg_schedule_id, }, qw( -integer -integer -integer -integer integer integer ); my @teams = map { bless $_, 'Teams' } ( { WT => 52, WD => 26, WH => 13, RS => 12, RA => 11, ID => 808 }, { WT => 52, WD => 27, WH => 16, RS => 200, RA => 100, ID => 909 }, { WT => 52, WD => 27, WH => 17, RS => 210, RA => 110, ID => 606 }, { WT => 52, WD => 27, WH => 17, RS => 210, RA => 111, ID => 303 }, ); print "\n"; $_->print_me for sort standings @teams; print "\n"; $_->print_me for sort_teams_by_standing_ST @teams; print "\n"; $_->print_me for sort_teams_by_standing_GRT @teams; print "\n"; $_->print_me for sort_teams_by_standing_SM @teams; print "\n"; $_->print_me for sort_teams_by_standing_SKM @teams; print "\n";
      Sort::Key and Sort::Maker are not core, but are almost as fast as the hand-coded transforms

      Actually, Sort::Key is faster than any other method. Only in some corner cases it could be surpassed by the GRT!

      That was a good bit of work and thorough; thanks a lot, I appreciate it. Cannot wait to try this at home...
Re: Refactor my sort
by salva (Canon) on Dec 23, 2006 at 22:58 UTC
    You can use Sort::Key::Maker to generate sorting subroutines easily:
    use Sort::Key::Maker sort_teams => # keys extraction sub: sub { ( $_->get_wins_total, $_->get_wins_division, $_->get_wins_home, $_->get_runs_scored, $_->get_runs_allowed, $_->get_reg_schedule_id ) }, # key types: qw(-int -int -int -int int int); ... for my $team (sort_teams @{$self->teams}) { ... }