First, you should turn on warnings. Had you done so, you would have noticed that you should use $row[1]
, $row[2]
... to access individual elements of @row
. @row[0]
creates a single element array slice.
Second, you would benefit from separating logic from presentation by using a templating module such as HTML::Template. Having while
loops mixed in with heredocs make for unreadable code.
You can access the parameter values passed to your script by using a CGI form processing package. These days, I prefer to use CGI::Simple rather than CGI.pm because CGI::Simple
has saner defaults and does not have the HTML generation baggage that comes with CGI.pm
.
use strict; use warnings;
use CGI::Simple;
my $cgi = CGI::Simple->new;
my $pname = $cgi->param('pname');
Finally, keep in mind that you still need to validate 'pid' even though it is specified as readonly
in the form. I would also recommend using CGI::Application to provide handlers for various actions rather than having a different script for each one.
Here is a simple example:
#!perl
use strict; use warnings;
use CGI::Simple;
use HTML::Template;
my $cgi = CGI::Simple->new;
# For demo purposes only. I would use CGI::Application
$cgi->param ? process_form($cgi) : show_form($cgi);
sub process_form {
my ($cgi) = @_;
print $cgi->header('text/plain'),
sprintf("%s : %s\n",
$cgi->param('pid'),
$cgi->param('pname'),
)
;
return;
}
sub show_form {
my ($cgi) = @_;
my $tmpl = HTML::Template->new(scalarref => template() );
$tmpl->param(
PRODUCTS => [
{ PID => '1234', PNAME => 'Widget' },
{ PID => '4321', PNAME => 'Wombat' },
]
);
print $cgi->header, $tmpl->output;
return;
}
sub template {
return \ <<EO_TMPL;
<!DOCTYPE HTML>
<html><head><title>Test</title></head>
<body>
<TMPL_LOOP PRODUCTS>
<form method="POST">
<p><input name="pid" readonly="1" value="<TMPL_VAR PID>">
<input name="pname" value="<TMPL_VAR PNAME>"><input
type="submit" value="Update"></p></form>
</TMPL_LOOP>
</body>
</html>
EO_TMPL
}