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

Here is my code. For the sake of discussion, the text file being opened contains 5 questions: 
#!/usr/bin/perl

use CGI qw(:standard);
use CGI::Carp qw(fatalsToBrowser);
use strict;
use warnings;
use vars qw($fullname $anslength @allselections $file $fname $lname $t
+ime $reviewtime $idinput $id $courselocation $useripaddress $question
+no @log $clocktime $log $type $value @answers $buttonname %selections
+ @options);
my @allquestions;
my @allanswers;
my @explain;

print header;
# ********************************* DEFINE VARIABLES
$time = param('time');
$reviewtime = param('reviewtime');
$file = param('file');
$idinput = param('idinput');
$fullname = param('fullname');
$fname = param('fname');
$lname = param('lname');
$courselocation = substr($file, 0, 6);
$useripaddress = $ENV{REMOTE_ADDR};
$questionno = 0;

print "<HTML><HEAD>";
print "<TITLE>Exam</TITLE>";
print "</HEAD>";

print qq/<BODY BGCOLOR=beige onLoad="begintimer()">/;

makeexam();

# ******************* START SUBROUTINE MAKEEXAM

sub makeexam() {
local $/ = "~\n";   # ** CHANGES INPUT RECORD SEPARATOR TO ~line break
# ************************** THIS SECTION CREATES THE HTML FOR THE EXA
+M.

chop($fname);
$fname = lc($fname);
$lname = lc($lname);
$fname = ucfirst($fname);
$lname = ucfirst($lname);

# ********************************** RECORD ACCESS TO THE EXAM
$clocktime = localtime;
@log = split(/\//, $file);
@log = split(/\./, $log[1]);
$log = $log[0];
$log = $log.".log";

open ACCESS, ">>$log" || die "Cannot open $log: $!";
print ACCESS $idinput . " " . $lname . " " . $fname . "  " . $clocktim
+e . " " . $useripaddress . " " . $time . "\n";
close(ACCESS);

# ********************************** END SCRIPT TO RECORD ACCESS TO TH
+E EXAM

print "<H2>Hello $fullname!</H2><P>";
print start_form (-name=>'makequiz', -method=>'POST', -action=>"http:/
+/ist221.nsm.tridenttech.edu/perl/gradequiz.cgi");

chdir("/perl/web");
open FILE, "$file" || die "Cannot open $file: $!";

while (<FILE>) {        # ** READS EACH LINE OF TEXT FROM FILE
chomp;                                # ** BY DEFAULT, CHOMPS OFF $/ F
+ROM EACH LINE
($type, $value) = split (/:#\s*/, $_);         # ***** SPLITS EACH LIN
+E BY TYPE AND CONTENT

if ($type =~ m/i/i) {        # ** CHECKS FOR IMAGE
print "<IMG SRC=http://ist221.tridenttech.edu/images/$courselocation/$
+value>";
print "<P>";
}elsif ($type =~ m/q/i) {        # ** CHECKS FOR QUESTION LINE & PRINT
+S QUESTION
   $questionno++;
   print "<B>$questionno. $value</B><BR>";
   push(@allquestions, $value);
}elsif ($type =~ m/a/i) {        # ** CHECKS FOR ANSWER LINE
        @answers = split (/`\s*/, $value);
        $anslength = @answers;                # ** LENGTH OF @ANSWERS
        push(@allanswers, $value);
        }elsif ($type =~ m/s/i) {                # ** CHECKS FOR SELEC
+TION LINE
           $buttonname = $type . $questionno;
                push(@allselections, $value);
                         %selections = split (/`\s*/, $value);
                 @options = keys (%selections);
                           if ($anslength > 1) {        # ** DETERMINE
+S TO USE RADIO OR CHECKBOX
                         print "<BR>";
                           print checkbox_group(-name=>$buttonname, -v
+alues=>\@options, -linebreak=>'true', -labels=>\%selections);
                           }else {
                         print "<BR>";
                         print radio_group(-name=>$buttonname, -values
+=>\@options, -linebreak=>'true', -labels=>\%selections, -default=>'-'
+);
                              }
                print "<BR><HR><BR>";
                }elsif ($type =~ m/e/i) {# ** CHECKS FOR EXPLAINATION 
+LINE
                        push (@explain, $questionno, $value);
                } # END ELSIF STATEMENTS
}   # ** END WHILE STATEMENT
close (FILE);

print "<CENTER>";

# ** THIS SECTION SENDS THREE ARRAYS TO THE GRADEQUIZ.CGI
print hidden(-name=>"answers", -default=>\@allanswers);
print hidden(-name=>"questions", -default=>\@allquestions);
print hidden(-name=>"selections", -default=>\@allselections);
print hidden(-name=>"explain", -default=>\@explain);
print hidden(-name=>'fname', -value=>$fname);
print hidden(-name=>'lname', -value=>$lname);
print hidden(-name=>'reviewtime', -value=>$reviewtime);
print hidden(-name=>'file', -value=>$file);
print hidden(-name=>'id', -value=>$idinput);

print submit (-value=>'Grade');
print "</CENTER>";
print end_form;
}

print "The last element in the questions array is $#allquestions<p>";
# ************************* END SUBROUTINE MAKEEXAM
print end_html;

[download]
I added the last print statement to help troubleshoot the problem. It says,

"The last element in the questions array is $#allquestions"

When I restart the web server and start fresh, the last element is 4. If I close out the quiz and then click the link to start the quiz again, the last element is -1. This continues until I restart my server again. I read about mod_perl and the arrays not re-initializing, but I can't figure out where I need to make my changes. I've tried a number of ways of initializing the variables, but can't seem to get it right. Can you shed some light on how I'm suppose to reinitialize the arrays and why perl is doing what its doing?

Also, I've noted what Ovid mentioned in my last posting about the security issue using param('file'), but I'll deal with that after I get the script running.

Many thanks.
Chris

Edit g0n - added readmore tags

Replies are listed 'Best First'.
Re: Can't seem to reinitalize arrays
by Joost (Canon) on Jul 12, 2005 at 16:55 UTC
    Ow! Ow! Ow!

    First, run this script through perltidy to clean up the formatting.

    Don't use global variables in mod_perl (or anywhere else, but especially in mod_perl) unless you have to. Use lexical (my) variables instead.

    While you're fixing that, make sure you define all your variables in the minimal scope you can, so they're not accidentally overwritten somewhere else in the script. You're using strict for a reason, right?

    I won't mention the $file thing, because you already have, but use taint mode in CGI scripts when you start writing them. Making a script secure is harder than making a secure script.

    Now, about your question: I suspect if you fix the problems above, the answer will become clear, or it will solve itself :-)

    update: since you've got warnings enabled, you will probably want to check the server's error log. the CGI to mod_perl porting guide is also very helpful in these instances.

    "What should it profit a man, if he should win a flame war, yet lose his cool?"
[reply]
[d/l]
[select]
      Ok, I ran the script through perltidy and eliminated the global variables, yet the number of questions in the questions array is still -1 when I access the script more than once. So, when the questions are numbered, instead of starting with 1, it begins with the next number after the last time the questions were displayed. What more can I do? Here is the cleaned up code: 
      #!/usr/bin/perl

use CGI qw(:standard);
use CGI::Carp qw(fatalsToBrowser);
# use strict;

use warnings;
# use diagnostics;

#use vars
#  qw($fullname $anslength @allselections $file $fname $lname $time $r
+eviewtime $idinput $id $courselocation $useripaddress $questionno @lo
+g $clocktime $log $type $value @answers $buttonname %selections @opti
+ons);

my (
     @explain,   @allanswers,     @allquestions,  $fullname,
     $anslength, @allselections,  $file,          $fname,
     $lname,     $time,           $reviewtime,    $idinput,
     $id,        $courselocation, $useripaddress, $questionno,
     @log,       $clocktime,      $log,           $type,
     $value,     @answers,        $buttonname,    %selections,
     @options
);

# my @allquestions;
# my @allanswers;
# my @explain;

print header;

# ********************************* DEFINE VARIABLES
$time           = param('time');
$reviewtime     = param('reviewtime');
$file           = param('file');
$idinput        = param('idinput');
$fullname       = param('fullname');
$fname          = param('fname');
$lname          = param('lname');
$courselocation = substr( $file, 0, 6 );
$useripaddress  = $ENV{REMOTE_ADDR};
$questionno     = 0;

print "<HTML><HEAD>";
print "<TITLE>Exam</TITLE>";
print "</head>";

# ******************* START SUBROUTINE MAKEEXAM

sub makeexam() {
    local $/ = "~\n";    # ** CHANGES INPUT RECORD SEPARATOR TO ~line 
+break

    # ************************** THIS SECTION CREATES THE HTML FOR THE
+ EXAM.

    chop($fname);
    $fname = lc($fname);
    $lname = lc($lname);
    $fname = ucfirst($fname);
    $lname = ucfirst($lname);

    # ********************************** RECORD ACCESS TO THE EXAM
    $clocktime = localtime;
    @log       = split( /\//, $file );
    @log       = split( /\./, $log[1] );
    $log       = $log[0];
    $log       = $log . ".log";

    open ACCESS, ">>$log" || die "Cannot open $log: $!";
    print ACCESS $idinput . " " . $lname . " " . $fname . "  "
      . $clocktime . " "
      . $useripaddress . " "
      . $time . "\n";
    close(ACCESS);

    # ********************************** END SCRIPT TO RECORD ACCESS T
+O THE EXAM

    print "<H2>Hello $fullname!</H2><P>";
    print start_form (
               -name   => 'makequiz',
               -method => 'POST',
               -action => "http://ist221.nsm.tridenttech.edu/perl/grad
+equiz.cgi"
    );

    chdir("/perl/web");
    open FILE, "$file" || die "Cannot open $file: $!";

    while (<FILE>) {    # ** READS EACH LINE OF TEXT FROM FILE
        chomp;          # ** BY DEFAULT, CHOMPS OFF $/ FROM EACH LINE
        ( $type, $value ) =
          split( /:#\s*/, $_ );    # ***** SPLITS EACH LINE BY TYPE AN
+D CONTENT

        if ( $type =~ m/i/i ) {    # ** CHECKS FOR IMAGE
            print
"<IMG SRC=http://ist221.tridenttech.edu/images/$courselocation/$value>
+";
            print "<P>";
        }
        elsif ( $type =~ m/q/i )
        {    # ** CHECKS FOR QUESTION LINE & PRINTS QUESTION
            $questionno++;
            print "<B>$questionno. $value</B><BR>";
            push( @allquestions, $value );
        }
        elsif ( $type =~ m/a/i ) {    # ** CHECKS FOR ANSWER LINE
            @answers = split( /`\s*/, $value );
            $anslength = @answers;    # ** LENGTH OF @ANSWERS
            push( @allanswers, $value );
        }
        elsif ( $type =~ m/s/i ) {    # ** CHECKS FOR SELECTION LINE
            $buttonname = $type . $questionno;
            push( @allselections, $value );
            %selections = split( /`\s*/, $value );
            @options = keys(%selections);
            if ( $anslength > 1 ) {    # ** DETERMINES TO USE RADIO OR
+ CHECKBOX
                print "<BR>";
                print checkbox_group(
                                      -name      => $buttonname,
                                      -values    => \@options,
                                      -linebreak => 'true',
                                      -labels    => \%selections
                );
            }
            else {
                print "<BR>";
                print radio_group(
                                   -name      => $buttonname,
                                   -values    => \@options,
                                   -linebreak => 'true',
                                   -labels    => \%selections,
                                   -default   => '-'
                );
            }
            print "<BR><HR><BR>";
        }
        elsif ( $type =~ m/e/i ) {    # ** CHECKS FOR EXPLAINATION LIN
+E
            push( @explain, $questionno, $value );
        }    # END ELSIF STATEMENTS
    }    # ** END WHILE STATEMENT
    close(FILE);

    print "<CENTER>";

    # ** THIS SECTION SENDS THREE ARRAYS TO THE GRADEQUIZ.CGI
    print hidden( -name => "answers",    -default => \@allanswers );
    print hidden( -name => "questions",  -default => \@allquestions );
    print hidden( -name => "selections", -default => \@allselections )
+;
    print hidden( -name => "explain",    -default => \@explain );
    print hidden( -name => 'fname',      -value => $fname );
    print hidden( -name => 'lname',      -value => $lname );
    print hidden( -name => 'reviewtime', -value => $reviewtime );
    print hidden( -name => 'file',       -value => $file );
    print hidden( -name => 'id',         -value => $idinput );

    print submit ( -value => 'Grade' );
    print "</CENTER>";
    print end_form;
}

# ************************* END SUBROUTINE MAKEEXAM
print end_html;

makeexam();

print "The last element in the questions array is $#allquestions<p>";

      [download]
[reply]
[d/l]

        You should probably work backwards. Following where you expect allquestions to get more items, and when you find that make sure it is working, if it isn't then back up another step until you get to the spot where something isn't happening that should be. If its your script and you uderstand it, you can always start at the begging and check that each case is happening as you expect it to. When you find that one variable or file that isn't behaving like you expect then give us that. It will help greatly.


        ___________
        Eric Hodges
[reply]
        What more can I do?
        You can try to narrow it down to a 5-6 line test case that shows us what isn't working like you expect. When you give us a 170 line chunk of code and say "fix this", it feels too much like we're doing your work without getting paid.
[reply]
Re: Can't seem to reinitalize arrays
by chriso (Sexton) on Jul 14, 2005 at 17:44 UTC
    I got it to work. My local variables were defined outside the subroutine. I didn't really need the subroutine anymore so I elimintated it and it worked.

    Thanks for all your input.

    Chris

[reply]

Back to Seekers of Perl Wisdom