Well, there is excellent stuff and terrible stuff there, and the code you posted doesn't match with the results you describe.

The good stuff is that you are using strictures, three parameter open and are reporting errors from the open statements very nicely. Well done!

The worst of the bad stuff is that your code is not indented at all! Aside from anything else that hides the next bad thing: you have nested your named subroutines inside other code. Perl lets you do that, but there is no good reason to do it (unlike some other languages). So the first thing is to move the subs out of the while loop and indent the code.

Having done that it becomes clear that there is a last statement at the end of events that can never be hit because it follows an unconditional return statement. Maybe that should be somewhere else, but it's not at all clear where.

The identifier records is used way too much. It is a subroutine name, it is an array, it is a scalar that holds a reference to an array and it looks like the array is used in two different incompatible roles as well. Very nasty indeed!

There are a bunch of other errors that I can't well advise on because I simply can't figure out what should really be happening. However, you might like to start with the following code, add some example data in place of the rubbish I've supplied, and see if you can reach a place where you can re-ask your question with something that has a better chance of working.

use strict;
use warnings;
my @records;
my @list;
my @sorted_recs;

while (<DATA>) {
    chomp;
    my @tokens = split;
    my @record = @tokens[3, 4, 5, 6, 10, 11];

    push @records, \@record;
}

print join (', ', @$_), "\n" for sort {$a->[3] <=> $b->[3]} @records;

__DATA__
0 1 2 3 4 5 6 7 8 9 10 11
1 1 2 2 4 5 2 7 8 9 10 11
2 1 2 1 4 5 9 7 8 9 10 11
[download]

Prints:

2, 4, 5, 2, 10, 11
3, 4, 5, 6, 10, 11
1, 4, 5, 9, 10, 11
[download]

Thank you for your advice.

Your code is very difficult to grasp for the following reasons. It is also failing for at least one of the following reasons:

• You have no indenting, which makes it really hard to visually grasp where loops, blocks, and subs start and end. This not only makes it difficult for us to comprehend, it makes it difficult for you, the programmer, to keep things straight.
• You are defining named subs within loops. This is a "bad idea." It probably doesn't do what you think it does. In fact, if your subs were rewritten to avoid using global osmosis, it's approximately the same as defining the sub just one time somewhere outside of the loop, and that would lead to clearer code too.
• You are passing values and variables into subs from higher lexical scopes (similar to using global variables to pass values into a sub). This makes readability very difficult, and in your case may be introducing a closure bug (see next item).
• Even though your subs are being defined within a loop, my assumption is that the 'my' variables available to them from outside the sub's brackets are the ones that occur on the first iteration of the loop. When that first iteration ends, the scope closes and all subsequent iterations will not work as you intend.

Pass all variables and values into your subs through @_, not through global osmosis. Define your subs outside of the loop. Your final use of 'last' seems to be useless.

I may be missing the mark on scope closing around the subs after the first iteration of the loop, but if I'm right about that, there's your biggest problem. One of the very few legitimate reasons to use a variable from a broader scope within a sub is to set up an intentional closure. I don't think you're intentionally doing that. Instead, it's an unintentional consequence of the way you've got your code structured.

Here is a simple example of the mistake you're making by defining the sub within a loop, and then using a loop-scoped variable as a global passing into the sub:

use strict; 
use warnings; 
use Modern::Perl;

for ( 1 .. 10 ) {
    my $iterator = $_;
    test();
    sub test {
        say $iterator;
    }
}
[download]

The result printed is '1' on every iteration. This is because the sub can only be defined one time. That one time it's defined all the variables available to it will be cemented into place. The definition can not change even though the loop *appears* to be re-defining it. This means that the first iteration of the loop creates a closure around the sub, and only those variables available to it on the first iteration will be available to it ever.




Dave

Thanks so much for your advice. I had no idea I was creating a closure around my sub.