Uninitialized value when moving sorting function to package

Question

Say I got a file main.pl and a package package.pm

In main.pl I got the function

sub GetTimestampFromDateStr {
        my $date_str = shift;

        $logger->debug("[GetTimestampFromDateStr]:(Checking date \"$date_str\"\n"); # Here

        my $unix_timestamp;
        if ( $date_str =~ /^\d{8}\.\d{6}$/ ) { # Here
            $unix_timestamp =
            Time::Piece->strptime( $date_str, '%Y%m%d.%H%M%S' )->epoch();
        }
        else {
            $logger->error( # Here
              "[GetTimestampFromDateStr]:(Invalid date format for \"$date_str\", must be \"%Y%m%d.%H%M%S\"  i.e. \"20170125.123500\"\n"
            );
        }

        return $unix_timestamp;
    }

which gets called by

sub DATESORT {
        my $TYPE = shift;

        my $ts_a = GetTimestampFromDateStr($a);
        my $ts_b = GetTimestampFromDateStr($b);

        if ( lc($TYPE) eq "desc" ) {
            $ts_b <=> $ts_a;
        }
        else {
            $ts_a <=> $ts_b;
        }
    } 

which is used as a sorting function & called via

sort { DATESORT('ASC') } @tran_dates_to_load

Everything works just fine when the functions and function calls are in the same file. When moving the function as-is to package.pm, it suddenly raises a bunch of Errors for uninitialized values.

Working with perl 5.16.3.

Answer 1

$a and $b are package variables, so cannot be easily accessed across package boundaries. Demonstration:

use v5.12;

package Foo {
    sub bar {
        say "[$a]";
        say "[$b]";
        1;
    }
}

package main {
    my @x = sort { Foo::bar() } 1 .. 2;
}

You can work around that by fully referencing the variables:

use v5.12;

package Foo {
    sub bar {
        say "[$main::a]";
        say "[$main::b]";
        1;
    }
}

package main {
    my @x = sort { Foo::bar() } 1 .. 2;
}

However, this hardcodes an assumption into Foo::bar() — it now assumes it will always be called from main.

You can eliminate the hardcoding thanks to caller, though it's a little ugly:

use v5.12;

package Foo {
    sub bar {
        no strict 'refs';
        my ( $x, $y ) = map ${ caller . '::' . $_ }, qw( a b );
        say "[$x]";
        say "[$y]";
        1;
    }
}

package main {
    my @x = sort { Foo::bar() } 1 .. 2;
}

A better solution is to pass $a and $b as function arguments.

use v5.12;

package Foo {
    sub bar {
        my ( $x, $y ) = @_;
        say "[$x]";
        say "[$y]";
        1;
    }
}

package main {
    my @x = sort { Foo::bar( $a, $b ) } 1 .. 2;
}

Answer 2

You're calling the somewhat expensive GetTimestampFromDateStr 2*n*log2(n) times when you only have n dates. This means that 16 dates results in 128 date conversions. That's a huge waste.

I recommend using Sort::Key to address this issue.

---

You are perfoming the lc($TYPE) eq "desc" check 2*n*log2(n) times when it only needs to be done once.

I recommend moving the check out of the compare function to address this issue.

---

use Sort::Key qw( ikeysort rikeysort );

my $sorter =
   ( lc($TYPE) eq "desc"
   ? sub { rikeysort { GetTimestampFromDateStr( $_ ) } @_ }
   : sub { ikeysort  { GetTimestampFromDateStr( $_ ) } @_ }
   );

my @sorted = $sorter->( @unsorted );

That said, there's no reason to convert the dates at all given the format they use!

my $sorter =
   ( lc($TYPE) eq "desc"
   ? sub { sort { $b cmp $a } @_ }
   : sub { sort { $a cmp $b } @_ }
   );

my @sorted = $sorter->( @unsorted );