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

Fellow Monasterians,

Spring is in the air, so time to dust off the golf clubs, or at least the refactoring tools.

I'm prepping an AoH that I'm pulling out of a database for use in a HTML::Template nested <tmpl_loop>. I'm getting the desired result, but my method (the for loop) seems a bit kludgy. So, is there anything that I can do to make this more orthodox or Perlish?

HTML in a .tmpl

<tmpl_loop case_list> <tmpl_var examiner> <tmpl_loop casedata> <tmpl_var examiner> <tmpl_var case> <tmpl_var location> </tmpl_loop> </tmpl_loop>

Perl

my $all_cases = [ #an AoH from a DBI query { 'examiner' => 'Brad', 'case' => '2345432', 'location' => 'Sandwich' }, { 'examiner' => 'Brad', 'case' => '7678979', 'location' => 'Wheaton' }, { 'examiner' => 'Mary', 'case' => '1234534', 'location' => 'Geneva' }, { 'examiner' => 'Mary', 'case' => '8966789', 'location' => 'Aurora' }, ]; my $case_list; my $temp = ""; my $cctr = -1; for my $i ( 0 .. $#$all_cases ) { if ( $all_cases->[$i]{'examiner'} ne $temp ) { $cctr++; $temp = $all_cases->[$i]{'examiner'}; $case_list->[$cctr]{'examiner'} = $temp; } push @{ $case_list->[$cctr]{'casedata'} }, $all_cases->[$i]; } $template->param( case_list => $case_list ); __OUTPUT__ [ { 'examiner' => 'Brad', 'casedata' => [ {'case' => '2345432', 'examiner' => 'Brad', 'location' => 'Sandwich'}, {'case' => '7678979', 'examiner' => 'Brad', 'location' => 'Wheaton'} ], } { 'examiner' => 'Mary', 'casedata' => [ {'case' => '1234534', 'examiner' => 'Mary', 'location' => 'Geneva'}, {'case' => '8966789', 'examiner' => 'Mary', 'location' => 'Aurora'} ], } ];

Thanks in advance.

—Brad
"The important work of moving the world forward does not wait to be done by perfect men." George Eliot

Replies are listed 'Best First'.
Re: Cleaner build of an AoH for nested loop?
by kyle (Abbot) on Mar 06, 2009 at 04:49 UTC

    I might write it this way.

    foreach my $case ( @{$all_cases} ) { if ( $case_list && $case->{examiner} eq $case_list->[-1]->{examiner} ) { push @{ $case_list->[-1]->{casedata} }, $case; } else { push @{$case_list}, { examiner => $case->{examiner}, casedata => [$case] }; } }

    That eliminates all the extra variables, and you're not looping over indices instead of elements, but it seems a little longer (even though it's really fewer statements).

      This was the eloquence I was looking for kyle, though the -1 list counters looked odd at first. But my original counters and looping just didn't feel "best practices."

      Thanks.

      —Brad
      "The important work of moving the world forward does not wait to be done by perfect men." George Eliot

        If the -1 index bothers you, you can build the list backwards. In the else clause, unshift instead of push, then you can use refer to the front of the list with [0] instead of the end with [-1]. Then when it's done, use reverse to get the original order. Given that, you have to decide if you want to have to take the extra step to reverse the list, or live with the odd look of [-1].

        foreach my $case ( @{$all_cases} ) { if ( $case_list && $case->{examiner} eq $case_list->[0]->{examiner} ) { push @{ $case_list->[0]->{casedata} }, $case; } else { unshift @{$case_list}, { examiner => $case->{examiner}, casedata => [$case] }; } } @{$case_list} = reverse @{$case_list};
Re: Cleaner build of an AoH for nested loop?
by ig (Vicar) on Mar 06, 2009 at 05:57 UTC

    Not sure about orthodoxy, but how about...

    my %cases; push(@{$cases{$_->{examiner}}}, $_) foreach(@$all_cases); $case_list = [ map { { examiner => $_, casedata => $cases{$_} } } keys %cases ];