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

Hello, fellow PerlMonks!

I joined back in March, but this is my first question to SoPW. A fellow monk suggested that I read Resorting to Sorting to get a handle on some alternate sorting idioms. While doing so, I'm having a spot of trouble with Exercise 2 in the Orcish Manuever section.

My code compiles cleanly, and it runs, but it's giving me incorrect results. The results I want are:

Unsorted List:beta, apple, gamma globulin, alpha, zeta, gesorenplatz!!
+, gamma, delta, foolish mortal,
Sorted List:, beta, zeta, alpha, apple, delta, gamma, foolish mortal, 
+gamma globulin, gesorenplatz!!

[download]
I don't want to spoil the exercise for those who haven't worked it yet, so I've spoilerized it...

My program: 
#!/usr/bin/perl -w
#
# Orcish_Ex.pl
#
# Exercises from the "Orcish Sorting" section of perlmonks tutorial no
+de
# "Resorting to Sorting" (id=node 128722).
#
# Exercise 1:
#       Q: Why should you make the caching hash viewable only by the
#       sorting function?  How is this accomplished?
#
#       A: To avoid namespace pollution.  By declaring locally in a bl
+ock.
#       (My answers.  I haven't checked 'em yet.)
#       NOTE: while coding it up, I started to suspect that %cache sho
+uld
#       be nulled out, or successive sort calls could botch it up.  Is
+ it
#       a problem?  See Orcish_Ex_test.pl
#
# Exercise 2:
#       Use Orcish Manuever to sort a list of strings in the same way 
+as
#       described in Naive section, Exercise 1.
#
# Note: Yes, there is an intentional blank line at the end so we can s
+ee
#       where the empty string will sort to!
#
use strict;
use warnings;

# Orcish section
{
        my %cache;
        sub len_or_str {
                my $va = ($cache{$a} ||= [ length($a), substr($a,0,5) 
+]);
                my $vb = ($cache{$b} ||= [ length($b), substr($b,0,5) 
+]);
                return  ($va->[0] <=> $vb->[0])
                        or ($va->[1] cmp $vb->[1]);
        }

        sub print_cache {
                print "Cache after sort:\n";
                for my $k (keys %cache) {
                        print "\t", $k, " => [", $cache{$k}->[0], ", "
+,
                                $cache{$k}->[1], "]\n";
                }
        }
}

sub print_msg_list {
        my $msg = shift @_;
        print $msg, join(", ", @_), "\n";
}

my @unsorted = <DATA>;
chomp @unsorted;

&print_msg_list("Unsorted List:", @unsorted);

my @sorted = sort len_or_str @unsorted;

&print_msg_list("Sorted List:", @sorted);

&print_cache;

__DATA__
beta
apple
gamma globulin
alpha
zeta
gesorenplatz!!
gamma
delta
foolish mortal

[download]
I'm certain it's something simple, but I can't see it. Any help?

UPDATE: I forgot to wrap the code in code tags!
UPDATE: I linked "Orcish Manuever" since planetscape told me how.

TIA, roboticus

Replies are listed 'Best First'.
Re: Sorting problem with Orcish Manuever
by japhy (Canon) on May 22, 2006 at 12:26 UTC
    It's your return() statement. You've got return (X) or (Y) which Perl reads as (return(X)) or (Y), and since return() ALWAYS returns, you never get to the second clause. Rewrite it as return ((X) or (Y)) or, in your specific case, return X or Y.

    And if you find any typos or errors otherwise in the article, please let me know. A problem I noticed looking at it right now is that the orcish maneuver should really be: 

    my $data_a = exists($cache{$a}) ? $cache{$a} : ($cache{$a} = mangle($a
+));
my $data_b = exists($cache{$b}) ? $cache{$b} : ($cache{$b} = mangle($b
+));

    [download]
    to allow for false and undefined values to be returned by mangle().
    Jeff japhy Pinyan, P.L., P.M., P.O.D, X.S.: Perl, regex, and perl hacker
    How can we ever be the sold short or the cheated, we who for every service have long ago been overpaid? ~~ Meister Eckhart
[reply]
[d/l]
[select]
      That's no longer "Orcish", is it? "Orcish" (for "or-cache, implies use of "||=". The fact that some stuff may return false and thus be recalculated, is something you have to weigh against the ease of coding.

      Usually, it's rather rare. In this case, you will never get a false value back. So it's completely unnecessary.

[reply]
      japhy:

      Thanks! I stared at that routine until I was blue in the face. I originally left the parentheses out, but had an entirely different bug in an earlier version. Then I left in the parentheses in 'cause I thought it looked good and wouldn't affect the value.
      <headsmack force="5">D'oh!</headsmack>

      --roboticus

[reply]
Re: Sorting problem with Orcish Manuever
by bart (Canon) on May 22, 2006 at 12:35 UTC
    japhy got it nailed, just before I did. But my solution is different: replace 
                    return  ($va->[0] <=> $vb->[0])
                        or ($va->[1] cmp $vb->[1]);

    [download]
    with 
                    return  ($va->[0] <=> $vb->[0])
                        || ($va->[1] cmp $vb->[1]);

    [download]
    and it works.

    p.s. Please don't make us hunt around for the question so much. When I finally found exercise 2 in that section, it simply referred me to another exercise in a different section! How hard is quoting a question of 2 lines?

[reply]
[d/l]
[select]
      bart:

      Thanks for the feedback. I'll give your version a try too.

      Regarding your P.S., sorry about that, my bad. I actually tried to link to it directly using syntax like [id://128722#the orcish manuever|Orcish Manuever], but it didn't work, and I couldn't figure out how to do it. I'll try to remember to add context in the future.

      Drat! And here I was patting myself on the back for actually reading the How (Not) To Ask A Question page (and others)... 8^P

      --roboticus

[reply]
[d/l]

        Just an FYI: To link to an anchor like you intended, use this syntax:

        [link://?node=128722#the%20orcish%20maneuver|The Orcish Maneuver]

        The above produces: The Orcish Maneuver :-)

        HTH,

        planetscape
[reply]
[d/l]
Re: Sorting problem with Orcish Manuever
by liverpole (Monsignor) on May 22, 2006 at 12:27 UTC
    You're getting exactly the results that one would expect, given that ...

    Read more... (784 Bytes)

    Update:  Never mind (must drink more coffee).  I actually tried the code, but "fixed" it inadverently (the way japhy describes above), then mistook the "expected results" for the actual results.  Forget I said anything.

    s''(q.S:$/9=(T1';s;(..)(..);$..=substr+crypt($1,$2),2,3;eg;print$..$/
[reply]
[d/l]
SUMMARY: Sorting problem with Orcish Manuever
by roboticus (Chancellor) on May 23, 2006 at 11:49 UTC
    Well, the problem turned out to be operator precedence in my return statement, as indicated by japhy and bart. Basically, using:

        return  $va->[0] <=> $vb->[0]
    ||      $va->[1] cmp $vb->[1];

    [download]
    or:

        return ($va->[0] <=> $vb->[0]
            or $va->[1] cmp $vb->[1]);

    [download]
    does the trick.

    japhy also pointed out an improved version of the Orcish Maneuver, while bart suggests that the improved version needs a new name, and that I should link to the subtopic so helpful PerlMonks don't have to chase wild geese in order to help me. And another thank you to planetscape for showing me how to properly link to a subtopic here.

    Thanks all!

    --roboticus

[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom