Maintainability of hash-sorting code

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

Greetings Monks,
I'm trying to sort a reasonably large amount of data stored in a hash, and I've come up with code that works. However, I just don't like the code - it looks like I'd curse the programmer that wrote it (me) if I came across it.
I don't think there's anything wrong with my algorithm, but I'd greatly appreciate any comments on the style and maintainability of this code. More idiomatic ways to write it (in particular the two lines concerning $idx and splice) would also be welcomed.

What I'm doing should (hopefully) be fairly obvious from the code and comments, but here's a general description anyway.

For each part number, sort so that the order numbers are in order, highest first
For each part number, find (at most) the five highest order numbers
Now display all the part numbers, ordering them by the order numbers

Note:
The data format isn't set in stone, and I'd be happy to change if a better alternative exists.

The code:

#!/usr/bin/perl

use warnings;
use strict;

#example data below.
my %data = (
        #Partno    List of orders
        "A1111" => [qw/12345 1234 123456 2345 233 23456/],
        "B2222" => [qw/12345/],
        "C3333" => [qw/12345 6789/],
        "D4444" => [qw/12345 1234 6789/],
        "E5555" => [qw/12345 1234 6789 99999 999999 444444 22222 111/]
+,
        "F6666" => [qw/888888 9999999 6789/],
);


#Find the 5 numerically largest orders for each partno, and display th
+em.

foreach my $part_no (sort keys %data) {
        my @matching_orders;

        #Sort each order by number, largest first..
        @matching_orders = sort { $a <=> $b } @{$data{$part_no}};

        #We want upto the most recent five. Nasty hack.
        my $idx = $#matching_orders-4;
        $idx = 0 if($idx < 0);
        my @top_five = reverse splice @matching_orders, $idx;
        $data{$part_no} = \@top_five;
}

#I now know the first element of each part_no array is
#the largest order number that part_no appears on.
#sort by that element.

foreach my $part_no (sort { ${$data{$a}}[0] <=> ${$data{$b}}[0] } keys
+ %data) {
        #Now display them
        print "Part Number \"$part_no\" was found on: ";
        print join ", ", @{$data{$part_no}};
        print "\n";
}
[download]

Thanks in advance,
davis

Comment on Maintainability of hash-sorting code Download Code

Replies are listed 'Best First'.
Re: Maintainability of hash-sorting code by Abigail-II (Bishop) on Jul 09, 2002 at 12:39 UTC
I would write your first loop differently. There's no need to sort the keys of the hash, as you are only modifying the values here. Secondly, since you are modifying just the values, I'd use `values` instead of `keys`. `reverse` in combination with `sort` is not really necessary, just reverse the condition in the block. Finally, don't splice if you can use a slice - specially not if you do it with a large detour the way you did. This is my loop: `foreach my $orders (values %data) { my @orders = sort {$b <=> $a} @$orders; $orders = [@orders [0 .. ($#orders < 5 ? $#orders : 4)]]; }` [download] Abigail	[reply] [d/l] [select]
Re: Maintainability of hash-sorting code by dragonchild (Archbishop) on Jul 09, 2002 at 13:53 UTC
Adding to what Abigail-II wrote ... (Welcome back, btw) What you're talking about is output and the transformations necessary to output in the appropriate fashion. Ideally, I would make sure that you're not being destructive to the data. Abigail-II's solution is destructive, because you can potentially lose information. (I.e., the 6th order number.) This may or may not be important to you. Maybe you've pulled the data from a database and can play with it in memory. Maybe you're done with that 6th order number and don't care about it anymore. I don't know. Just be careful. ------ We are the carpenters and bricklayers of the Information Age. Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.	[reply]