Search code examples
perlsortinginheritancemethodssubroutine

Why isn't my sort working in Perl?


I have never used Perl, but I need to complete this exercise. My task is to sort an array in a few different ways. I've been provided with a test script. This script puts together the array and prints statements for each stage of it's sorting. I've named it foo.pl:

use strict;
use warnings;
use MyIxHash;

my %myhash;
my $t = tie(%myhash, "MyIxHash", 'a' => 1, 'abe' => 2, 'cat'=>'3');
$myhash{b} = 4;
$myhash{da} = 5;
$myhash{bob} = 6;

print join(", ", map { "$_ => $myhash{$_}" } keys %myhash) . " are the starting key => val pairs\n";

$t->SortByKey;  # sort alphabetically
print join(", ", map { "$_ => $myhash{$_}" } keys %myhash) . " are the alphabetized key => val pairs\n";

$t->SortKeyByFunc(sub {my ($a, $b) = @_; return ($b cmp $a)});  # sort alphabetically in reverse order
print join(", ", map { "$_ => $myhash{$_}" } keys %myhash) . " are the reverse alphabetized key => val pairs\n";

$t->SortKeyByFunc(\&abcByLength);  # use abcByLength to sort
print join(", ", map { "$_ => $myhash{$_}" } keys %myhash) . " are the abcByLength sorted key => val pairs\n";

print "Done\n\n";


sub abcByLength {
  my ($a, $b) = @_;

  if(length($a) == length($b)) { return $a cmp $b; }
  else { return length($a) <=> length($b) } 
}

Foo.pl uses a package called MyIxHash which I've created a module for called MyIxHash.pm. The script runs through the alphabetical sort: "SortByKey", which I've inherited via the "IxHash" package in my module. The last two sorts are the ones giving me issues. When the sub I've created: "SortKeyByFunc" is ran on the array, it passes in the array and a subroutine as arguments. I've attempted to take those arguments and associate them with variables.

The final sort is supposed to sort by string length, then alphabetically. A subroutine for this is provided at the bottom of foo.pl as "abcByLength". In the same way as the reverse alpha sort, this subroutine is being passed as a parameter to my SortKeyByFunc subroutine.

For both of these sorts, it seems the actual sorting work is done for me, and I just need to apply this subroutine to my array.

My main issue here seems to be that I don't know how, if possible, to take my subroutine argument and run my array through it as a parameter. I'm a running my method on my array incorrectly?

package MyIxHash;
#use strict;
use warnings;
use parent Tie::IxHash;
use Data::Dumper qw(Dumper);

sub SortKeyByFunc {
    #my $class = shift;
    my ($a, $b) = @_;

    #this is a reference to the already alphabetaized array being passed in
    my @letters = $_[0][1];

    #this is a reference to the sub being passed in as a parameter
    my $reverse = $_[1];

    #this is my variable to contain my reverse sorted array
    my @sorted = @letters->$reverse();

    return @sorted;
}

1;

Solution

  • "My problem occurs where I try: my @sorted = @letters->$reverse(); I've also tried: my @sorted = sort {$reverse} @letters;"

    You were really close; the correct syntax is:

    my $reverse = sub { $b cmp $a };
    # ...
    my @sorted = sort $reverse @letters;
    

    Also note that, for what are basically historical reasons, sort passes the arguments to the comparison function in the (slightly) magic globals $a and $b, not in @_, so you don't need to (and indeed shouldn't) do my ($a, $b) = @_; in your sortsubs (unless you declare them with a prototype; see perldoc -f sort for the gritty details).


    Edit: If you're given a comparison function that for some reason does expect its arguments in @_, and you can't change the definition of that function, then your best bet is probably to wrap it in a closure like this:

    my $fixed_sortsub = sub { $weird_sortsub->($a, $b) };
    
    my @sorted = sort $fixed_sortsub @letters;
    

    or simply:

    my @sorted = sort { $weird_sortsub->($a, $b) } @letters;
    

    Edit 2: Ah, I see the/a problem. When you write:

    my @letters = $_[0][1];
    

    what you end up with a is a single-element array containing whatever $_[0][1] is, which is presumably an array reference. You should either dereference it immediately, like this:

    my @letters = @{ $_[0][1] };
    

    or just keep is as a reference for now and dereference it when you use it:

    my $letters = $_[0][1];
    # ...
    my @sorted = sort $whatever @$letters;
    

    Edit 3: Once you do manage to sort the keys, then, as duskwuff notes in his original answer, you'll also need to call the Reorder() method from your parent class, Tie::IxHash to actually change the order of the keys. Also, the first line:

    my ($a, $b) = @_;
    

    is completely out of place in what's supposed to be an object method that takes a code reference (and, in fact, lexicalizing $a and $b is a bad idea anyway if you want to call sort later in the same code block). What it should read is something like:

    my ($self, $sortfunc) = @_;
    

    In fact, rather than enumerating all the things that seem to be wrong with your original code, it might be easier to just fix it:

    package MyIxHash;
    use strict;
    use warnings;
    use parent 'Tie::IxHash';
    
    sub SortKeyByFunc {
        my ($self, $sortfunc) = @_;
    
        my @unsorted = $self->Keys();
    
        my @sorted = sort { $sortfunc->($a, $b) } @unsorted;
    
        $self->Reorder( @sorted );
    }
    
    1;
    

    or simply:

    sub SortKeyByFunc {
        my ($self, $sortfunc) = @_;
    
        $self->Reorder( sort { $sortfunc->($a, $b) } $self->Keys() );
    }
    

    (Ps. I now see why the comparison functions were specified as taking their arguments in @_ rather than in the globals $a and $b where sort normally puts them: it's because the comparison functions belong to a different package, and $a and $b are not magical enough to be the same in every package like, say, $_ and @_ are. I guess that could be worked around, but it would take some quite non-trivial trickery with caller.)

    (Pps. Please do credit me and duskwuff / Stack Overflow when you hand in your exercise. And good luck with learning Perl — trust me, it'll be a useful skill to have.)