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

Dear Perlmonks--
I humbly request assistance with somewhat trivial matters.

I'm trying to do a bunch of stuff with a list of cgi parameters. What I'm trying to do here 
use CGI;
use Text::CSV_XS;

my %param;
foreach my $field ($cgi->param) {
    $param{ $field } = $cgi->param( $field );
    $param{ $field } =~ s/^\s+|\s+$//g; 
}

my $outfile = './work_request.csv';
if ($param{mode} eq 'submit') {
    my $result = check_all();
    if ($result eq 'ok') {
        $csv = Text::CSV_XS->new({
            'always_quote' => 1
        });
        @request = ($param{title}
                   ,$param{requester}
                   ,$param{department}
                   ,$param{date_requested}
                   ,$param{proposed_completion}
                   ,$param{emergency}
                   ,$param{work_around}
                   ,$param{situation}
                   ,$param{solution}
                   ,($param{B01} || 0)
                   ,($param{B02} || 0)
                   ,($param{B03} || 0)
                   ,($param{B04} || 0)
                   ,($param{B05} || 0)
                   ,($param{B06} || 0)
                   ,$param{benefits});
        $csv->combine(@request);
        my $line = $csv->string;
        # $csv->print($outfile, $line) # this is how I should be writi
+ng the file
                                       # but it demands an arrayref (?
+)
        my $fh = new IO::File ">> $outfile";
        if (defined $fh) {
            print $fh ($line);
            $fh->close;
        }
    }
}

sub check_all {
    $result = 'ok' if ($param{title}
                      ,$param{requester}
                      ,$param{department}
                      ,$param{date_requested}
                      ,$param{proposed_completion}
                      ,$param{emergency}
                      ,$param{work_around}
                      ,$param{situation}
                      ,$param{solution}
                      ,($param{B01}
                            ||
                        $param{B02}
                            ||
                        $param{B03}
                            ||
                        $param{B04}
                            ||
                        $param{B05}
                            ||
                        $param{B06})
                      ,$param{benefits})
}

[download]
is check to make sure all parameters are present, including one of B01-B06, then write CSV to a file. I don't understand why $result always gets assigned the value "ok", and I can't get the output in a file.

Also, how can I incorporate a complex data structure, ie: putting all the params in a hash of hashes and run tests on that?

Replies are listed 'Best First'.
Re: Easy way to run tests on many CGI params?
by chromatic (Archbishop) on Aug 31, 2000 at 01:28 UTC
    I'm a little paranoid when I deal with CGI parameters. Sometimes I do something like the following (note the or test is untested -- I usually get precedence wrong the first time): 
    my @expected = qw( title requester department etc );
my @request;
foreach my $expect (@expected) {
    push @request, ($q->param($expect) or '0');
}

    [download]
    What you could do is put the B1 .. B6 parameters in a separate array and do the or 0 trick on them. Put the other parameters in the first array and do something like this: 
    foreach my $expect (@expected) {
    my $received = $q->param($expect);
    unless (defined $received) {
        $error .= "Didn't get $expect parameter!\n";
        last;        # and do an error somewhere
    }
    push @request, $received;
}

    [download]
    This way, you don't have a malicious HTML hijacker pass bad data and have it end up in your file somewhere. Plus, you get to check immediately.

    Update: isotope makes a good point below. A little client-side JavaScript is good for verifying submissions, but some users have it disabled (for good reason), and my point about not trusting the client still applies. Just be friendly about it, as isotope recommends.

[reply]
[d/l]
[select]
      In the second foreach loop, omitting the last; and utilizing $error:
      foreach my $expect (@expected) {
       my $received = $q->param($expect);
       unless (defined $received) {
           $error .= "Didn't get $expect parameter!\n";
       }
       push @request, $received;
   }
if($error ne "") {
       # Handle the error here, reprinting the form,
       # pre-filled with what they've already sent.
       # Also tell them about all the problems:
       print $error;
}

      [download]

      Users who don't understand "required field" tend to get really frustrated when they're only told about the first omission, because it takes them numerous iterations to get rid of all the errors.


      --isotope
[reply]
[d/l]
Re: Easy way to run tests on many CGI params?
by ncw (Friar) on Aug 31, 2000 at 01:25 UTC
    The if statement in check_all is wrong - you are using the comma operator in the expression - probably not what you intended. This will return true provided $param{benefits} is set. If you replace the ,s with || then I expect it will work. (Update: or even && as geektron suggests below :-)

    You can put all the parameters into a hash by using the param() method of the CGI module in list context. This returns you all the parameter names. Eg something like 

      my %hash = map { $_ => $cgi->param($_) } $cgi->param();

    [download]
[reply]
[d/l]
      But, don't do this in general, because multi-value params lose the ability to pull up the multivalues. You've just regressed into what cgi-lib.pl got oh-so-wrong, and one of the things that CGI.pm fixes properly.

      -- Randal L. Schwartz, Perl hacker

[reply]
        If you were worried about this you could collect the parameters like this :- 
          my $hash = {};
  for ($cgi->param())
  {
    push @{$hash->{$_}}, $cgi->param($_);
  }

        [download]
        However I find that 9 times out of 10 you want the simpler hash, so you can collect the data twice in a slightly more complicated way something like this :- 
          my $hash = {};
  for ($cgi->param())
  {
    push @{$hash->{list}->{$_}}, $cgi->param($_);
    $hash->{scalar}->{$_} = $hash->{list}->{$_}->[0];
  }

        [download]
        You can then access most data with a simple $hash->{scalar}->{'whatever'} but for those cases when you cared about multiple entries you can do $hash->{list}->{'whatevers'}->1

        I think the CGI module is fantastic, but I do find it much more convenient to have my data in a hash rather than have to call it from param() when I need it. For a CGI of any length then I'd certainly use one of another of the methods above.

[reply]
[d/l]
[select]
RE: Easy way to run tests on many CGI params?
by geektron (Curate) on Aug 31, 2000 at 02:29 UTC
    i agree that the if statement in sub check_all is incorrect, but it looks more like you want to use && for testing (because it looks like you're requiring all except the Bxx variables)
[reply]
Re: Easy way to run tests on many CGI params?
by wardk (Deacon) on Aug 31, 2000 at 19:07 UTC

    Although it's more code, I would suggest treating all the fields individually. Especially if you desire to have some fields be guaranteed numeric or some other format that might be required of a database. In this case you are using a CSV file, but I notice that one field is 'date_requested". It might be desireable to ensure that this date is not only a real date, but formatted in a particular way, so you have consistency.

    In the case where a field is not validated, rather than report that error and continuing after the user resubmits that correction, I would suggest checking all fields and on every error, push an error message into a simple array. Then you can report all the errors to the end user looping through the error array and allow then to fix all potential problems with the form in one pass.

    good luck!

[reply]

Back to Seekers of Perl Wisdom