Improve this "group by" perl sub

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

Hi monkers,

First post here. I'm using perl professionally for more than a year now but I'm only doing simple tasks. I'm always impressed when I come here because I don't understand half the code I read... and to improve myself I would like to know how you would have done the function posted below.

It's a simple function that I created to do "group by" on rows retrieved by fetchall_arrayref({});

I'm particulary interessed in:

- better style

- performance improvement

- or more black magic ;)

Here my .pl :

#!/usr/bin/perl
use strict;
use Data::Dumper;

my $rows = [    {'Category' => 'Animal',  'Type' => 'Dog', 'Model' => 
+'Labrador',   'Quantity' => 5},
                {'Category' => 'Animal',  'Type' => 'Dog', 'Model' => 
+'Rottweiler', 'Quantity' => 2},
                {'Category' => 'Animal',  'Type' => 'Cat', 'Model' => 
+'Chartreux',  'Quantity' => 4},
                {'Category' => 'Vehicle', 'Type' => 'Car', 'Model' => 
+'Ford',         'Quantity' => 10},
            ];
            
my $groupedRows = rowsGroupBy($rows, ['Category', 'Type'], { 'SUM' => 
+['Quantity'] });

print $groupedRows->{'Animal', 'Dog'}->{'Quantity'};
# 7

print "\n";
print Dumper $groupedRows;
# $VAR1 = {
          # 'AnimalCat' => {
                            # 'Type' => 'Cat',
                            # 'Quantity' => 4,
                            # 'Category' => 'Animal'
                          # },
          # 'VehicleCar' => {
                             # 'Type' => 'Car',
                             # 'Quantity' => 10,
                             # 'Category' => 'Vehicle'
                           # },
          # 'AnimalDog' => {
                            # 'Type' => 'Dog',
                            # 'Quantity' => 7,
                            # 'Category' => 'Animal'
                          # }
        # };

        
# $rows: Array of hash_ref (rows)
# $groupBy: Array of field to use for the group by. Can be empty if yo
+u want to do $options on all rows.
# $options: Hash with following option handled:
#            SUM => @array: Compute the sum.
#            PILE => @array: Create an array containing all values for
+ the field.
#            MAX => @array: The greatest value encountered.
# Return hash_ref with group by as keys (list) if group by is not empt
+y.
# Return hash_ref with $options as keys if group by is empty.
sub rowsGroupBy {
    my($rows, $groupBy, $options) = @_;
    
    return undef if @$rows+0 == 0 || !defined($groupBy);
    
    my $result = {};
    
    foreach my $row (@$rows) {
        my $value;
        my $key = join(chr(28), map { $row->{$_} } @$groupBy);
        
        if(exists($result->{$key})) {
            $value = $result->{$key};
        }
        else {
            $value = {};
            $result->{$key} = $value;
        }
        
        foreach(@$groupBy) {
            $value->{$_} = $row->{$_};
        }
        foreach(@{$options->{'SUM'}}) {
            $value->{$_} += $row->{$_};
        }
        foreach(@{$options->{'PILE'}}) {
            push(@{$value->{$_}}, $row->{$_});
        }
        foreach(@{$options->{'MAX'}}) {
            $value->{$_} = $row->{$_} if !defined($value->{$_}) || $ro
+w->{$_} gt $value->{$_};
        }
        if($options->{'KEEP_UNDERLYING_ROWS'}) {
            push(@{$value->{'UNDERLYING_ROWS'}}, $row);
        }
    }
    return $result->{''} if @$groupBy == 0;
    return $result;
}
[download]

Comment on Improve this "group by" perl sub Download Code

Replies are listed 'Best First'.
Re: Improve this "group by" perl sub by roboticus (Chancellor) on Feb 17, 2011 at 11:33 UTC
seblec: To be honest, I would normally do it in the SQL, letting it do the grunt work of grouping the data, finding the max and sum: `select category\|\|type as grp, max(category) as cat, max(type) as typ, sum(quantity) as qty from table group by category \|\| type` [download] On the perl side, I think you're trying to make rowsGroupBy too "smart". Rather than doing one task well, you're trying to make it a jack-of-all trades. I would think that a simple function that would return a list of records with the group-by keys would be a single function I'd use, something like: `sub group_by { my ($rows, $group_keys) = @_; my @recs=(); for my $row (@$rows) { push @recs, { GRP=>join(chr(28),@{$row}{@$group_keys}), %$row }; } return @recs; }` [download] Similarly, you could write other single-purpose functions to handle MAX and SUM, or use functions from List::Util and List::MoreUtils to do the other jobs. ...roboticus When your only tool is a hammer, all problems look like your thumb.	[reply] [d/l] [select]

Replies are listed 'Best First'.

Re: Improve this "group by" perl sub
by roboticus (Chancellor) on Feb 17, 2011 at 11:33 UTC

seblec:

To be honest, I would normally do it in the SQL, letting it do the grunt work of grouping the data, finding the max and sum:

select category||type as grp, max(category) as cat, max(type) as typ,
       sum(quantity) as qty
from table
group by category || type
[download]

On the perl side, I think you're trying to make rowsGroupBy too "smart". Rather than doing one task well, you're trying to make it a jack-of-all trades. I would think that a simple function that would return a list of records with the group-by keys would be a single function I'd use, something like:

sub group_by {
    my ($rows, $group_keys) = @_;
    my @recs=();
    for my $row (@$rows) {
        push @recs, {
            GRP=>join(chr(28),@{$row}{@$group_keys}),
            %$row 
        };
    }
    return @recs;
}
[download]

Similarly, you could write other single-purpose functions to handle MAX and SUM, or use functions from List::Util and List::MoreUtils to do the other jobs.

...roboticus

When your only tool is a hammer, all problems look like your thumb.

[reply]
[d/l]
[select]