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


This sort takes a hash of hashes called %nodes (yes, this is something of an E2 project). Each element in %nodes is a hash - the key is the node's title, and its elements are attributes.

I'm sorting on the node's creation time, which is given in the form "YYYY-MM-DD HH:MM:SS"

The subroutine splits the two dates/times into the appropriate particles, then converts that to seconds-since-the-epoch time and compares.

As you can see, it's pretty long. It works, and that's all that matters, but I'm trying to do some learning over here - I want to shrink it.

At boo_radley's encouragement, I submit the subroutine to the Monks. How small can you get it?
Thanks...
if($sort_field eq 'createtime') { for $node (sort { ($date, $time) = split / /, $nodes{$a}{createtime}; ($year, $month, $day) = split /-/, $date; ($hour, $min, $sec) = split /:/, $time; $create_timeA = timelocal($sec,$min,$hour,$day,$month,$year); ($date, $time) = split / /, $nodes{$b}{createtime}; ($year, $month, $day) = split /-/, $date; ($hour, $min, $sec) = split /:/, $time; $create_timeB = timelocal($sec,$min,$hour,$day,$month,$year); if($sort_order eq 'ASC') { return $create_timeA <=> $create_timeB; } else { return $create_timeB <=> $create_timeA; } } keys %nodes) { print "$node: $nodes{$node}{$sort_field}\n"; } } else { whatever; }
---
donfreenut

Replies are listed 'Best First'.
Re: Shrink this sort subroutine
by merlyn (Sage) on Apr 24, 2001 at 22:32 UTC
    Well, first off, your use of timelocal is broken. And I wouldn't bother doing that anyway... you already have sortable strings!
    my @node_list = sort { $nodes{$a}{createtime} cmp $nodes{$b}{createtim +e} } keys %nodes;

    -- Randal L. Schwartz, Perl hacker


      Well, that certainly is shorter...

      My mistake when I tried that earlier was using '<=>' instead of 'cmp.' Probably should have been suspicious of a string comparison with a non-alphanumeric operator...

      Argh.

      Thanks, all...

      ---
      donfreenut
Re: Shrink this sort subroutine
by tadman (Prior) on Apr 24, 2001 at 23:09 UTC
    Just as a remark, you might want to use UNIX time_t format instead of this ASCII-ified version YYYY-MM-DD etc. If you use time_t, you can convert between time zones, and allow for "Daylight Savings Time" settings. If everything is set to GMT, of course, which it should be by default. time_t is what the time function spits out by default.

    Then, provided you are doing numeric comparisons, you can sort them with ease. This is super-ultra important, because time_t is rolling over to 10 digits and you don't want to fall victim to The Y2.001775K Bug.

    Until September 9, 2001, you can get away with sorting using the default string method. However, any dates after that point will sort back into 1973 territory, which is the bad kind of retro.

    So, you can sort them like so:
    sub bycreatetime { return $nodes{$a}{createtime} - $nodes{$b}{createtime}; } foreach $node (sort { bycreatetime } keys %nodes) { # ... }
    As an added bonus, I see that you're comparing your '$sort_field' variable. Consider the following improvement:
    my (%sort_method) = ( createtime => \&bycreatetime, createuser => \&bycreateuser, # For example ); foreach $node (sort $sort_method{$sort_field} keys %nodes) { # ... }
    Or, for continued amusement:
    sub NumericalSort { my ($field) = @_; return sub { return $nodes{$a}{$field} - $nodes{$a}{$field}; }; } my (%sort_method) = ( createtime => NumericalSort('createtime'), ); foreach $node (sort $sort_method($sort_field) keys %nodes) { # ... }
      ++! Thanks for pointing that out!

      Just in case some other people (like I did) gasp and think that everything time is going to roll over to 000000000 on Sept 9th, it won't it will become 1000000000, its just sorting dates without taking this into account will end up with this phenomenon:
      042312313 (Apr 15 1970) 1028542120 (Aug 5 2002) 171024424 (Jun 3 1975) 514542121 (Apr 22 1986) 941244212 (Oct 30 1999) 974124124 (Nov 13 2000) 987451241 (Apr 16 2001) 988145259 (Apr 24 2001)
      You can probably see the error. So be careful. Thanks tadman

      Update: Hmm, I should have followed your link, before posting this. I blame the caffeine!

      $ perldoc perldoc
Re: Shrink this sort subroutine
by suaveant (Parson) on Apr 24, 2001 at 22:30 UTC
    dude! as long as you 0 pad your numbers, a plain sort will work on those dates!

    Update Sorry, just realized a plain sort won't quite work, but a simple hash sort should...
                    - Ant


      Um, unless I misunderstand your definition of 'zero pad,'I tried that first, and it didn't work. The numbers are already zero-padded to the YYYY-MM-DD HH:MM:SS format.

      Am I misinterpreting?

      ---
      donfreenut
        '2000-02-23 01:30:06' would be an example of zero padded... '2000-2-23 1:30:6 would be non zero padded
                        - Ant
Re: Shrink this sort subroutine
by suaveant (Parson) on Apr 24, 2001 at 22:35 UTC
    BTW, you could also just split your dates on /\D+/, since the date and time is all numbers...

    and you can always use reverse for descending searches....
                    - Ant