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

Maybe it's that it's 9:00 pm on a Friday and I'd rather be at home cooking some dinner and sipping a beer, but I just can't figure this one out:

Below is HTML::Mason code that generates a list of users within each group

The array ref is populated by a SQL query that I've verified returns all of the relevant information in the format with users grouped by group_name.

$_->[0] = group_id $_->[1] = group_name $_->[2] = user_name

So what I'm trying to do is output the group name only once (in a table) for all of the users who belong to that group.

% for (my $i = 0; $i < @{$all_groups}; $i++) { % my $j = $i; Start loop: $i=<% $i %>, $j=<% $j %><br> % while ($all_groups->[$i][0] == $all_groups->[$j][0]) { % % # for ($j = $i; $all_groups->[$i][0] == $all_groups->[$j][0]; $j++ +) { % die if ($j > 2000); $i=<% $i %>, $j=<% $j %>, user=<% $all_groups->[$j][2] %><br> % $j = $j + 1; % } After loop: $i=<% $i %>, $j=<% $j %><br> % $i = $j; End loop: $i=<% $i %>, $j=<% $j %><br> % }

Minus the Mason crap it's this:

for (my $i = 0; $i < @{$all_groups}; $i++) { my $j; print STDOUT "Start loop: \$i=$i, \$j=j\n"; for ($j = $i; $all_groups->[$i][0] == $all_groups->[$j][0]; $j++) +{ die if ($j > 2000); # Code to keep Apache from crashing as +Perl runs amuck from my various solutions print STDOUT "\$i=$i, \$j=$j, user=" . $all_groups->[$j][2]. " +\n"; } print STDOUT "After loop: \$i=$i, \$j=$j\n"; $i = $j; }

What's happening, however, is that the loop skips a number at most but not all breaks between groups. In effect, an extra iteration is occuring during the for{} loop that means the test fails but $j gets incremented by 1. Now I can't see why this code isn't doing what it's supposed to, so maybe I've beens staring at it for too long, but I'd really appreciate it if anyone can point out my mistake.

TIA

P.S. Here's some sample output:

Start loop: $i=0, $j=0 $i=0, $j=0, user=mash@4anything.com $i=0, $j=1, user=rblaustein@4anything.com $i=0, $j=2, user=sstarr@4anything.com $i=0, $j=3, user=lzogby@4anything.com $i=0, $j=4, user=sshankman@4anything.com $i=0, $j=5, user=rnafziger@4anything.com $i=0, $j=6, user=smondeaux@4anything.com $i=0, $j=7, user=rkaiser@4anything.com $i=0, $j=8, user=mkarsch@4anything.com $i=0, $j=9, user=katkins@4anything.com $i=0, $j=10, user=sgreene@4anything.com $i=0, $j=11, user=arabschnuk@4anything.com $i=0, $j=12, user=klange@4anything.com $i=0, $j=13, user=vs@4anything.com $i=0, $j=14, user=msinger@4anything.com $i=0, $j=15, user=csultanik@4anything.com $i=0, $j=16, user=cweber@4anything.com $i=0, $j=17, user=earbuckle@4anything.com $i=0, $j=18, user=tgill@4anything.com $i=0, $j=19, user=jgrishaver@4anything.com $i=0, $j=20, user=mkost@4anything.com After loop: $i=0, $j=21 End loop: $i=21, $j=21 Start loop: $i=22, $j=22 $i=22, $j=22, user=dan_derosa@es.adp.com $i=22, $j=23, user=finlay_waugh@es.adp.com $i=22, $j=24, user=deborah_barron@es.adp.com After loop: $i=22, $j=25 End loop: $i=25, $j=25 Start loop: $i=26, $j=26 $i=26, $j=26, user=kirshenbaumda@aetna.com $i=26, $j=27, user=poseyll@aetna.com $i=26, $j=28, user=dubejw@aetna.com $i=26, $j=29, user=flanaganm1@aetna.com $i=26, $j=30, user=wallra@aetna.com After loop: $i=26, $j=31 End loop: $i=31, $j=31 Start loop: $i=32, $j=32 $i=32, $j=32, user=alice.stroup@aig.com

Replies are listed 'Best First'.
Re (tilly) 1: Nested For Loop giving me extra iterations
by tilly (Archbishop) on Dec 09, 2000 at 07:35 UTC
    The problem is that in the third part of your for loop you are incrementing $i, and in your main loop you are assigning what you want it to be.

    For the record I would never think of using a double-loop to solve this problem. Instead I might build a hash of arrays like this (untested) code:

    my %group2user; foreach my $rec (@all_groups) { push @{$group2user{$rec->[0]}}, $rec; } foreach my $group_id (sort keys %group2user) { # Do whatever }
    or else I would assume that the groups appeared grouped and work like this:
    my $old_id; for my $rec (@all_groups) { if (not defined $old_id or $old_id <=> $rec->[0]) { # Spit out header $old_id = $rec->[0]; } # Print user record }
    With either approach I push work down to Perl, and wind up with more reliable code. :-)

      After a good night's sleep and some thought I did come up with a way to do what I wanted using a single for loop. The reason that I had to keep working on it, depsite tilly's code is that the output wasn't quite what I needed...

      To define the issue a little better -- I am outputting tables, and for the sake of rendering speed, it would be faster to output only a single row for each group with a single cell containing all the users who belong to that group. tilly's second snippet, while clean, can only (at least, as far as I can figure it) easily output a row per user... although I could simply output the group name once by testing against the old_id to see when I'd moved into a new group.

      Here's what the output should look like in pseudo HTML:

      <table> <row> <cell>Group 1 name <cell>Email 1, Email 2, Email 3, Email 4... <row> <cell>Group 2 name <cell>Email 5, Email 6, Email 7... </table>
      <cell>

      The reason I wanted to avoid the hash-based solution was that my data is already ordered by the SQL query I'm doing, so it seems strange to stuff ordered information into an 'unordered' hash only to have to sort it again.

      So, here's what I finally came up with:

      print <<EOTableHeaders; <table border="1" cellpadding="3" cellspacing="4"> <tr> <td class="list"><b class="list">Group Name</b></td> <td class="list"><b class="list">Group Members</b></td> <td class="list"><b class="list">Edit Mail Group</b></td> </tr> EOTableHeaders for (my $i = 0, my $j = 0; $i < (scalar(@{$all_groups}) - 1); $i++, $j +++) { print "<tr"; if ($j % 2 == 0) { print qq|bgcolor="#AAAACC"|; } print ">\n"; print qq|<td valign="top"><span class="list">| . $all_groups->[$i] +[1] |</span></td>\n|; print qq|<td valign="top"><span class="list">|; while ((defined($all_groups->[($i + 1)][0])) && ($all_groups-> +[$i][0] == $all_groups->[($i + 1)][0])) { print $all_groups->[$i][2] . "<br>"; $i++; } print $all_groups->[$i][2]; print "</td>\n"; print qq|<td><a href="edit_group.html?group_id=| . $all_groups->[$ +i][0] . |&&group_name=| . Apache::Util::escape_uri($all_groups->[$i][ +1]) |>Edit this Email Group</a></td>\n|; print "</tr>"; } </tr> </table>

      Or, for a slightly cleaner version:

      for (my $i = 0; $i < (scalar(@{$all_groups}) - 1); $i++) { print "Group name: " . $all_groups->[$i][1] . "\n"; while ((defined($all_groups->[($i + 1)][0])) && ($all_groups->[$i] +[0] == $all_groups->[($i + 1)][0])) { print "\tEmail: " . $all_groups->[$i][2] . "\n"; $i++; } print "\tEmail: " $all_groups->[$i][2] . "\n"; }
Re: Nested For Loop giving me extra iterations
by merlyn (Sage) on Dec 09, 2000 at 22:32 UTC
    You should be using a hash for your group list. I think it was Larry who said it first, but the phrase "arrays are for ordering, hashes are for searching" comes to mind. If you find yourself searching an array, or wishing the hash were ordered, you probably did the wrong thing. There are exceptions, but rare.

    -- Randal L. Schwartz, Perl hacker