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

This is a little project I started with the idea of trying to teach myself a little about CGI. It seemed to be going okay until I hit this snag. I'm not sure which information will be helpful so I'm going to err on the side of verbosity.

My script is creating a table in HTML based on a series of hashes read in from text files. The idea being that if the items in the table change, I just change the text file instead of the code. I am writing each table row by means of a foreach loop on an array list of the items. Here's a snippet:

foreach (@inventory) { print "<TR>"; print "<TD>$_</TD>"; print "<TD>$inv_prices{$_}</TD>"; print "<TD>$inv_weights{$_}</TD>"; print "<TD><input type=\"text\" size=\"4\" name=\"quantity$_\" ></ +TD></TR>" }

OK, my problem is with "quantity$_". That's the field that the user would actually change. When you enter a quantity into the field and press the SUBMIT button, the next script can't seem to recognize it as a parameter unique to a given item and prints nothing in the "shopping cart." Here's the code snippet from the 2nd script:

foreach (@inventory) { my $qty = param("quantity$_"); $qty{$_} = $qty } foreach (@inventory) { print "<TR>"; print "<TD>$_</TD>"; print "<TD>$inv_prices{$_}</TD>"; print "<TD>$qty{$_}</TD></TR>"; }

Nothing comes out on the page, though. If I leave out the code relevant to "quantity$_" and the %qty hash, then the rest formats fine.

How do I set up the parameter for the quantity field in the first script so that it changes for each item and get the 2nd script to read that parameter properly? Or am I just going about this in completely the wrong way? Thanks.

Replies are listed 'Best First'.
Re: CGI problem: trying to assign parameters in a loop
by chromatic (Archbishop) on Jan 19, 2002 at 22:48 UTC
    Here's a wild (untested) idea for the second script:
    my %quantities = map { (my $name = $_) =~ s/^quantity//; $name => param($_); } grep { /^quantity/ } param(); ;
    I think there may be a better way to represent your data as HTML form widgets, but it's not coming to mind immediately.
Re: CGI problem: trying to assign parameters in a loop
by trs80 (Priest) on Jan 20, 2002 at 00:00 UTC
    While I agree with gav^ for the most part I also think that some people maybe forced to work with old code or maybe only need a simple (famous last words) "one time" solution. The example code given is not readable and makes too many calls to the system. It is difficult to "read" because the "'s have to be escaped and it has to make a lot of system calls because of the duplicated print commands.
    So I would rewrite the loop to look like this:
    foreach (@inventory) { print qq~ <TR> <TD>$_</TD> <TD>$inv_prices{$_}</TD> <TD>$inv_weights{$_}</TD> <TD><input type="text" size="4" name="quantity$_" ></TD> </TR> ~; }

Re: CGI problem: trying to assign parameters in a loop
by bjelli (Pilgrim) on Jan 20, 2002 at 00:01 UTC

    The problem must be somewhere else in your script. I just cut-n-pasted your code, and got it to run in 2 minutes. Here's my complete code:

    #!/usr/bin/perl use CGI ('param'); print "Content-type: text/html\n\n"; print "<html><h1>Testing bigharas inventory problem</h1>\n"; @inventory = qw( camel llama ); if (param('action') eq 'submit') { print "Reading Data...<br>\n"; foreach (@inventory) { my $qty = param("quantity$_"); $qty{$_} = $qty } print "<table border=1>\n"; foreach (@inventory) { print "<TR>"; print "<TD>$_</TD>"; print "<TD>$inv_prices{$_}</TD>"; print "<TD>$qty{$_}</TD></TR>"; } print "</table>\n"; print "That's all\n"; } else { print "<form><table border=1>\n"; foreach (@inventory) { print "<TR>"; print "<TD>$_</TD>"; print "<TD>$inv_prices{$_}</TD>"; print "<TD>$inv_weights{$_}</TD>"; print "<TD><input type=\"text\" size=\"4\" name=\"quantity$_\" + ></TD></TR>" } print "</table><input type=submit name=action value=submit>\n"; }
    --
    Brigitte    'I never met a chocolate I didnt like'    Jellinek
    http://www.horus.com/~bjelli/         http://perlwelt.horus.at
Re: CGI problem: trying to assign parameters in a loop
by nandeya (Monk) on Jan 20, 2002 at 03:51 UTC
    I pretty much copied and pasted as well and came to the same conclusion as bjelli in her comment.
    Just added a few things on real quick to make an inclusive script as well and was able to run without issue.

    #!/usr/bin/perl -w use strict; use CGI; my $q = new CGI; my $subbut = $q->param("submitbut"); my @inventory = qw(books paper pencils erasers); print $q->header; print $q->start_html(); my %qty=(); my %inv_prices=(); my %inv_weights=(); if ($subbut eq 'Submit') { foreach (@inventory) { my $qty = $q->param("quantity$_"); $qty{$_} = $qty; } print "<br><br><TABLE>"; foreach (@inventory) { print "<TR>"; print "<TD>$_</TD>"; print "<TD>$qty{$_}</TD></TR>"; } print "</TABLE>"; } else { print "<form><table>\n"; foreach (@inventory) { print "<TR>"; print "<TD>$_</TD>"; print "<TD>$inv_prices{$_}</TD>"; print "<TD>$inv_weights{$_}</TD>"; print "<TD><input type=\"text\" size=\"4\" name=\"quantity$_\"></T +D></TR>\n"; } print "<TR>"; print "<TD><input type=\"submit\" name=\"submitbut\" value=\"Submit\ +"></TD>\n"; print "<TD><input type=\"reset\" value=\"Clear\"></TD></TR></table>< +/form>\n"; } print $q->end_html;


    nandeya
Re: CGI problem: trying to assign parameters in a loop
by gav^ (Curate) on Jan 19, 2002 at 23:06 UTC

    I'll just make the comment that embedding HTML in perl scripts is BAD.

    Seriously with templating solutions such as HTML::Template and Template-Toolkit there is no need to do it.

    Update: tutorial for TT here and a comparison of various templating systems here.

    gav^

      Sigh.

      Yes, templating is a nice solution to a number of problems, and can really clarify your code. But keep in mind it is not always bad. Sometimes a quick-and-dirty solution is easier. Many people like Perl because it enables you to do things that would otherwise take hours to do.

      • Object orientation is always better
        In many cases, OO is just a pain in the ass, and a functional interface is better. I really like perl 6's ability to create your own operators.
      • Embedding text in scripts is always wrong
        A quick-and-dirty solution can save both time and money. Besides, there's nothing wrong with embedding just a little. In situations where overloaded servers have to be used, templating can be too slow.
      • Prototypes should never be used
        Tilly told me why prototypes are bad. And they sure are. But sometimes, they're good. The (&@) for creating map-like syntax, or the () prototype to let perl inline your sub (great for constants).
      • Calling external programs when perl can do it is about as wrong as it can get
        There's the quick-and-dirty approach again. Although wrong in many production projects, it can speed up development a lot if you're creating a one-time hack, or a simple sysadmin script.
      • XML is the #1 way to store data
        XML is a hype. XML is a nice format to store data in, but there's a lot of ways to do it. I personally dislike pipeline seperated files, but even those can be quite good.
      • Never use a regex to grab the filename out of a path
        ($foo = $bar) =~ s!.*/!! is easier than using File::BaseName. The code's not portable, but not all code has to be.
      • Death to Dot Star!
        Dot star has a function. If you know what it does, and it does what you want, USE IT.
      • Etcetera etcetera
        Many things, including templating are overrated. Some developers (including yours truly) forget there are other solutions that can be as good, or maybe even better in particular cases.
      Maybe you're right about this script. Maybe _this_script_ should use a templating solution. Add "in most cases", "in this case" or something like that and you're right.

      Keep in mind: TIMTOWTDI, and some ways TDI are better than others, but that doesn't mean the worse ways should be avoided completely.

      2;0 juerd@ouranos:~$ perl -e'undef christmas' Segmentation fault 2;139 juerd@ouranos:~$

        If it is not a throw away script then you need to attempt at least some good programming practices.

        In my experiance I've found that too many simple hacked together CGI scripts stay in production because a) they work and b) nobody has the time/money to fix them. The problem is that 6 months down the line somebody normally has to make a small change, which turns out to be another hack and so on and so forth.

        Sometime you have to sit down and say, lets make this work properly and do it in a way that allows simple maintanance.

        I have a big problem with embedded HTML in CGI scripts, I might be the one that has to maintain it and then I've got to spend ages trying to work out how it works.

        Plus in his code he used the normal perl quoting which means he has to remember to put a \ in front of each quote which is a royal pain in the arse.

        What's wrong with:

        use File::Basename; $foo = basename($bar);
        It's a hell of a lot more obvious what this does than your rexexp.

        gav^