Beefy Boxes and Bandwidth Generously Provided by pair Networks
go ahead... be a heretic

Re: Confessions of an Initiate: a first Perl program

by Chmrr (Vicar)
on Nov 21, 2002 at 20:33 UTC ( #214904=note: print w/replies, xml ) Need Help??

in reply to Confessions of an Initiate: a first Perl program

I'd second most of the suggestions that Thesus and fruiture offered. Furthermore, I'd comment that as this is a purely linear script, there are few advantages to using subroutines other than the visual partitioning. The downside is that it promotes either the use of globals, as you did, or rediculous amounts of variable passing. And globals, as we all know "are always bad."{1} As such, were I writing this, I would do away with the subroutines and just deliniate the sections with comments.

Also, you hard-code the values of the popup box -- why, when you can get at them far easier by using keys %all_quotas? This will also prevent errors caused by typos, as well as allowing for expansion if you add more lines of data. The same concept applies to the default value of the popup.

Another tip is that most of the lines of process_query boil down to:

sub process_query { $period = $query->param('period') || (keys %all_quotas)[0]; print_table; }

Note the use of the || operator to set a default -- that's one of its primary uses. Lastly, the gigantic paragraph that you put in quotes looks ripe for being a HERE document.

You can see the agglomeration of the changes here, and the result here. Ignore how I removed your __DATA__ section -- I had to do so because they don't play nice with mod_perl. It is an excellent and appropriate use of __DATA__, but not one which works with mod_perl, unfortunatly.

{1} I hate generalizations like this. But in this case, it's usually good advice. There are times to use 'em, but here we don't need or want 'em.

perl -pe '"I lo*`+$^X$\"$]!$/"=~m%(.*)%s;$_=$1;y^`+*^e v^#$&V"+@( NO CARRIER'

Replies are listed 'Best First'.
Re: Re: Confessions of an Initiate: a first Perl program
by mooseboy (Pilgrim) on Nov 21, 2002 at 21:46 UTC

    Wow, many thanks to all who responded so far! I'll have to spend a while to try and synthesize all the thoughtful suggestions and see if I can come up with an improved version.

    Cheers, mooseboy

Re: Re: Confessions of an Initiate: a first Perl program
by mooseboy (Pilgrim) on Nov 22, 2002 at 07:49 UTC

    Chmrr, many, many thanks for your "native Perl" rewrite of my effort! One question: in your version, the dates in the pop-up menu are not ordered -- is there a way to order them so that the most recent is first, then the next most recent, etc, as in the original?

    <Thanks, mooseboy

      Not to sound cliche or anything, but you must first understand the cause of the problem; then the solution will become obvious. The reason the popup ordering isn't in order is because it is gotten through keys %some_hash -- which, as we know, produces said keys in no (easily) predictable or useful order. While it is possible that one could attempt to re-sort the list that it returns into some useful order, the current date format looks to make that rather hard.

      Given that, what we really want is to keep track of the keys of the hash as we add them, in some structure which preserves order -- say, like an array or something..

      Thus, we do is have an @dates array (declared next to where we define %quotas, say), and push $period onto the end of @dates as we go though loading the data. Later on, we can use @dates instead of keys %quotas if the ordering matters.

      I've intentionally not provided much explicit code here. Consider it an "excercise left to the reader." ;>

      perl -pe '"I lo*`+$^X$\"$]!$/"=~m%(.*)%s;$_=$1;y^`+*^e v^#$&V"+@( NO CARRIER'

        I see ... thanks for the hint, that'll keep me busy for a while!

        Done it! Of course, you did give me just a teensy-weensy little hint, Chmrr... many thanks! Here's the final (?) version:

        #!/usr/bin/perl use warnings; use strict; use CGI; ## Chmrr's revision, very slightly modified ## Column headings my @members = qw/ Algeria Indonesia Iran Iraq Kuwait Libya Nigeria Qat +ar Saudi_Arabia UAE Venezuela total_OPEC /; @members = map { tr/_/ /; $_ } @members; ## Load data my (%quotas, @dates); while (<DATA>) { ( my $period, $_ ) = split ':'; push @dates, $period; @{$quotas{$period}}{@members} = split ' '; } ## Introductory paragraph my $query = CGI->new(); print $query->header("text/html"), $query->start_html( -title => "QuotaBase: a database of OPEC oil p +roduction quotas", -bgcolor => "#cbcbcb" ), $query->h1("Welcome to QuotaBase!"), $query->table( $query->Tr( $query->td({-width => "600"}, $query->p(<<EOP) QuotaBase is an interactive database of OPEC oil production quotas. By + default, the <b>current quotas</b> are displayed. To view <b>histori +cal quota information</b>, select the period you want from the drop-d +own list and click the 'Show quotas' button. A table of all the quota +s for that period will be displayed. EOP ) ) ), $query->start_form(), "Choose a period: ", "&nbsp;", $query->popup_menu( -name=>'period', -values=>[@dates], -default=>(@dates)[0]), "&nbsp;&nbsp;", $query->submit(-name=>'submit', -value=>'Show quotas'), "&nbsp;&nbsp;", $query->defaults('Reset current quotas'), $query->endform; ## Data table my $period = $query->param('period') || (@dates)[0]; print $query->table({-border=>1}, $query->Tr( $query->td({-colspan=>2,-align=>"center +"},"<b>$period</b>") ), map { $query->Tr( $query->td({-width=>130,-align=>"left +" }, $_), $query->td({-width=>130,-align=>"righ +t"}, $quotas{$period}{$_}) ) } sort keys %{$quotas{$period}} ); print $query->end_html; __DATA__ Jan 02 - Dec 02: 693 1125 3186 0 1741 1162 1787 562 7053 1894 2 +497 21700 Sep 01 - Dec 01: 741 1203 3406 0 1861 1242 1911 601 7541 2025 2 +670 23201 Apr 01 - Aug 01: 773 1255 3552 0 1941 1296 1993 627 7865 2113 2 +786 24201 Feb 01 - Mar 01: 805 1307 3698 0 2021 1350 2075 653 8189 2201 2 +902 25201 31 Oct 00 - Jan 01: 853 1385 3917 0 2141 1431 2198 692 8674 2333 3 +077 26700 1 Oct 00 - 30 Oct 00: 837 1359 3844 0 2101 1404 2157 679 8512 2289 3 +019 26200

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://214904]
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others cooling their heels in the Monastery: (5)
As of 2021-04-16 03:14 GMT
Find Nodes?
    Voting Booth?

    No recent polls found