How to write this code correctly ??? Please help

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

Hi all, I have a report in flat text file report.data. I would like to scan serial number which I have and compare with the report. Here is what I did: In my html file.

<html><head><title>Scanning and compare </title></head>
<body bgcolor=white>
<BR><CENTER>
<form action="compare.cgi" method="POST">
Scan serial number:
<input type="text" name="name" size=30>
<input type="submit" value="Search">
</form>
</CENTER>

</body>
</html>
[download]

and here is my compare.cgi

#!/usr/local/perl 
use Time::Local;
$today = timelocal(localtime);
$now = &unix_to_date($today - 3600);

sub unix_to_date {

         my ($date) = $_[0]; # reads in variable passed to subroutine 
+reates array of month names
         my (@months) = qw!01 02 03 04 05 06 07 08 09 10 11 12!; #c
         
         @time = (localtime($date))[3,4,5,0,0]; #creates array of date
+ from unix time
         
         $time[2] += 1900; # converts year to proper format
         return "$months[$time[1]]/$time[0]/$time[2] ";

         }

$datafile = "report.data";

read(STDIN, $buffer, $ENV{'CONTENT_LENGTH'});
@pairs = split(/&/, $buffer);
foreach $pair (@pairs) {
    ($name, $value) = split(/=/, $pair);
    $value =~ tr/+/ /;
    $value =~ s/%([a-fA-F0-9][a-fA-F0-9])/pack("C", hex($1))/eg;
    $value =~ s/~!/ ~!/g;
    $FORM{$name} = $value;
}

$searchstr = $FORM{'name'};

open(INF,$datafile);
@mydata = <INF>;
close(INF);
print "Content-type:text/html\n\n";
print "<html><head><title>Check Report</title></head>\n";
print "<body><center><h3>Check Report</h3></center>\n";

@results = grep(/$searchstr/i,@mydata);
if ($#results >= 0) {
    foreach $i (@results) {
        chomp($i);
        ($sn , $Location,  $starttime, $endtime, $item, $family, $stat
+us) = split(/|/,$i);
         
        print "<center><h2><font color=red> $FORM{'name'} This serial 
+is in  Report List </font></h2></center>\n";
        
            open(OUTF,">>found_it.txt") or dienice("Couldn't open foun
+d_it.txt for writing: $!");

            # This locks the file so no other CGI can write to it at t
+he 
            # same time...
            flock(OUTF,2);
            # Reset the file pointer to the end of the file, in case 
            # someone wrote to it while we waited for the lock...
            seek(OUTF,0,2);

            print OUTF "$FORM{'name'}\|$item\|\|$now\n";
    }
} else {
    print "<center><h2><font color=green> $FORM{'name'} . Alert! This 
+serial number is not in  Report list, Please check.</font></h2></cent
+er>\n";
    open(OUTF,">>found_it.txt") or dienice("Couldn't open found_it.txt
+ for writing: $!");

            # This locks the file so no other CGI can write to it at t
+he 
            # same time...
            flock(OUTF,2);
            # Reset the file pointer to the end of the file, in case 
            # someone wrote to it while we waited for the lock...
            seek(OUTF,0,2);

            print OUTF "$FORM{'name'}\|$item\|NA\|$now";
}
print "</body></html>\n";
[download]

Somehow i can not write $item in report.data into found_it.txt can you help me rewrite this script so i can learn it. Thank you very much,

Comment on How to write this code correctly ??? Please help Select or Download Code

Replies are listed 'Best First'.
Re: How to write this code correctly ??? Please help by chromatic (Archbishop) on Oct 07, 2002 at 19:42 UTC
The problem is in this line: `split(/\|/,$i);` In a regular expression, the vertical bar is a metacharacter used to specify alternation. The regex asks for nothing or nothing. You should rather escape the pipe character so it is treated literally. (I find it strange that you escape it within a double-quoted string but not the regex. :) `split(/\\|/, $i);` There are several other opportunities to clean your code. Using strict and enabling warnings will force you to be more careful about variable usage. Using the CGI module will provide you with a better way of parsing input.	[reply] [d/l] [select]
Re: How to write this code correctly ??? Please help by kabel (Chaplain) on Oct 07, 2002 at 19:38 UTC
first: use strict and/or use warnings are two recommended pragmas that you should put on the head of all your scripts you write. i think i do not need to explain why ;) then consider to use the CGI.pm package. the parameter passing for example is very straightforward with it (have a look at the function Vars). you should unlock the file at the end of the script. locks are AFAIK a set bit inside the inode of the file. i do not know what exactly dienice () does - perhaps you should use CGI::Carp qw(fatalsToBrowser); at the top of the cgi; fatal errors become visible in your browser with it. hope that helped a bit.	[reply]
Re: How to write this code correctly ??? Please help by Flexx (Pilgrim) on Oct 07, 2002 at 21:12 UTC
Apart from what was said, you might want to use strftime() from the POSIX module, or CPAN's Date::Format to do your date/time conversions conveniently. Update: Of course, there's also Date::Manip (pure Perl) which also has a very intelligent input parser, and Date::Calc (XS/C library), which is faster. So long, Flexx	[reply]
A reply falls below the community's threshold of quality. You may see it by logging in.