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

Fellow monks:
First of all, the code here actually works. But can it be done better? (You might want to skip this node unless you're a golfer or efficiency nut).

Desc: each database record contains a name with their city and state. There can be multiple records with the same state and city; e.g., 10 in Texas with 5 in Dallas, 2 in Fort Worth, 3 in Waco, etc. At this point in the script, I only want to show each unique state with unique city names underneath; e.g., 1 heading for Texas, with 1 listing each for Dallas, Fort Worth, and Waco under it. You know the drill.

Like I said, it works. But code feels amateurish, like using the $temp_ variables to hold the first instance of a state or city. I've read nodes here on PM using grep and map to normalize arrays, and checked the Perlfaqs, but not sure how those methods would work with the hash or would be any cleaner. Comments? Ideas? Tee-time? Thanks.
while (@data = $sth->fetchrow_array()) { if ($data[1] ne $tempstate) { undef (@cities); push (@cities, $data[0]); $states{$data[1]} = [ @cities ]; $tempstate = $data[1]; $tempcity = $data[0]; } elsif ($data[0] ne $tempcity) { push (@cities, $data[0]); $states{$tempstate} = [ @cities ]; $tempcity = $data[0]; } }
For the entire script:
#!/usr/bin/perl print "Content-type: text/html\n\n"; use warnings; use CGI::Carp qw(fatalsToBrowser); use strict; my ($dbh, $sth, $stmt, $tempstate, $tempcity, %states, @data, @cities, + $city, $x); &dbconnect; #connect to database subroutine $stmt = "SELECT city, state, country FROM sponsor ORDER BY country, + state, city"; $sth = $dbh->prepare($stmt) or die "prepare: $stmt: $DBI::errstr"; $sth->execute() or die"execute: $stmt: $DBI::errstr"; while (@data = $sth->fetchrow_array()) { if ($data[1] ne $tempstate) { @cities = ""; push (@cities, $data[0]); $states{$data[1]} = [ @cities ]; $tempstate = $data[1]; $tempcity = $data[0]; } elsif ($data[0] ne $tempcity) { push (@cities, $data[0]); $states{$tempstate} = [ @cities ]; $tempcity = $data[0]; } } for my $state ( keys %states ) { print "$state:<br />"; for my $i (1..$#{ $states{$state} } ) { print "$i = $states{$state}[$i]<br />"; } } $sth->finish(); $dbh->disconnect();

—Brad
"A little yeast leavens the whole dough."

Replies are listed 'Best First'.
•Re: Better code for normalizing a list for a HoA?
by merlyn (Sage) on Jan 17, 2004 at 21:05 UTC
    Why not just use autovivification? Replace all that code with:
    push @{$states{$data[0]}}, $data[1];
    And let Perl handle the initial empty arrayref and subsequent lookup? What do you think this is, Java? {grin}

    -- Randal L. Schwartz, Perl hacker
    Be sure to read my standard disclaimer if this is a reply.

      Thanks merlyn, but I'm a little embarrassed. I just got P.O.R.&M (the Alpaca book) and was only on Chap 3. Autovivification is in Chap 4 {grin}.

      Anyway for anyone actually following this thread $data[0] and $data[1]need to be switched in meryln's clever one-liner.

      I tried the code and it worked first time, but it does not delete duplicate cities, only duplicate states. So, in my example, 'Dallas' would show up 5 times under 'Texas'.

      Well, on to Chap 4 of the Alpaca book to see how this can be remedied.

      —Brad
      "A little yeast leavens the whole dough."
        Oh, then you want nested hashes for automatic set reduction:
        $states{$data[1]}{$data[0]} = 1; ... for my $state (sort keys %states) { print "$state: ", join(", ", sort keys %{$states{$state}}), "\n"; }
        Yes, read the book faster!

        -- Randal L. Schwartz, Perl hacker
        Be sure to read my standard disclaimer if this is a reply.