Link to home
Start Free TrialLog in
Avatar of KenHeckert
KenHeckert

asked on

Review Script Please

The below script will be used to edit a news article in a flat file database.  The contents of the file are:

24|Kenny|EWPlanet Finale!|12:36 AM PST|3/04/2003|article|1
23|Kenny|EWPlanet Opens!|12:26 AM PST|3/04/2003|article|1
22|Kenny|Article Edited|12:36 AM PST|3/04/2003|The article that was edited|1
21|Kenny|EWPlanet Opens!|12:26 AM PST|3/04/2003|article|1
20|Kenny|EWPlanet Finale!|12:36 AM PST|3/04/2003|article|1
19|Kenny|EWPlanet Opens!|12:26 AM PST|3/04/2003|article|1
18|Kenny|EWPlanet Finale!|12:36 AM PST|3/04/2003|article|1
17|Kenny|EWPlanet Opens!|12:26 AM PST|3/04/2003|article|1

Last night the save portion of the script was working but it no longer is so I'm wondering what I might have did wrong.  If you can review the code, test it out on your website, and help me debug it... life would be great. ;)
_________________________________________________________

#!/usr/bin/perl

  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;
  }

#\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\

print "Content-type: text/html \n\n";

 $lnum = $FORM{'lnum'};
 $num = $FORM{'num'};

 $news_author = $FORM{'news_author'};
 $news_email = $FORM{'news_email'};
 $news_date = $FORM{'news_date'};
 $news_time = $FORM{'news_time'};
 $news_userid = $FORM{'news_userid'};
 $news_num = $FORM{'news_num'};
 $news_title = $FORM{'news_title'};
  $news_title =~ s/\cM//g;
  $news_title =~ s/\n\n/<p>/g;
  $news_title =~ s/\n/<br>/g;
  $news_title =~ s/&lt;/</g;
  $news_title =~ s/&gt;/>/g;
  $news_title =~ s/&quot;/"/g;
 $news_article = $FORM{'news_article'};
  $news_article =~ s/\cM//g;
  $news_article =~ s/\n\n/<p>/g;
  $news_article =~ s/\n/<br>/g;
  $news_article =~ s/&lt;/</g;
  $news_article =~ s/&gt;/>/g;
  $news_article =~ s/&quot;/"/g;

#\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\

 $qs = $FORM{'qs'};

   if ($qs eq "") { &choose; }
   if ($qs eq "choose") { &choose; }
   if ($qs eq "edit") { &edit; }
   if ($qs eq "save") { &save; }

#\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\

sub choose {

 open(ENEWS, "DB.txt");
  @enews = <ENEWS>;
 close (ENEWS);

 print qq~
  <html>
  <form method=POST action=edit.cgi>
  <input type=hidden name=qs value=edit>
 ~;

 foreach $line (@enews){
 chomp($line);
  ($lnum,$author,$title,$time,$date,$article,$userid)=split(/\|/,$line);
  if ($userid eq '1')
  {
 print qq~<div style="text-align: justify; font-family: verdana; font-size: 10pt;"><input type="radio" name="num" value="$lnum" class="radio"> $title on $date @ $time</font><br></div>\n~;
  }
 }
 print qq~
<div align=center>
<table border=0 cellspacing=0 cellpadding=0 class=fields_con>
 <tr>
  <td colspan=2><center><input type=submit value="edit news article"></center></td>
 </tr>
</table>
 </form>
 </html>
 ~;
}

#\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\

sub edit {

  print qq~
 <html>
 Edit Screen
 ~;

 open(ENEWS, "DB.txt");
  @enews = <ENEWS>;
 close (ENEWS);

 print qq~
  <html>
  <form method=POST action=edit.cgi>
  <input type=hidden name=qs value=save>
 ~;

 foreach $line (@enews){
 chomp($line);
  ($nnum,$author,$title,$time,$date,$article,$userid)=split(/\|/,$line);
  if ($nnum eq $num)
  {
print qq~
<div style="text-align: justify; font-family: verdana; font-size: 10pt;">
<input type=text name=news_num value="$nnum"><br>
<input type=text name=news_author value="$author"><br>
<input type=text name=news_title value="$title"><br>
<input type=text name=news_time value="$time"><br>
<input type=text name=news_date value="$date"><br>
<textarea COLS=53 ROWS=10 name=news_article>$article</textarea><br>
<input type=text name=news_userid value="$userid"><br>
</div>
~;
  }
 }
 print qq~
<div align=center>
<table border=0 cellspacing=0 cellpadding=0>
 <tr>
  <td colspan=2><center><input type=submit value="save news article"></center></td>
 </tr>
</table>
 </form>
 </html>
 ~;


}

#\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\

sub save {
print qq~
<html>\n
start script
<p>\n\n
$news_num<br>
$news_author<br>
$news_title<br>
$news_time<br>
$news_date<br>
$news_article<br>
$news_userid<br>
~;

$form_line = "$news_num";
$form_author = "$news_author";
$form_title = "$news_title";
$form_time = "$news_time";
$form_date = "$news_date";
$form_article = "$news_article";
$form_userid = "$news_userid";

open (DB,"<DB.txt") || die "cant open DB:$!";
@array = <DB>;
close (DB);

foreach (@array)
{
    chomp $_;
    my ($art_num, $art_author, $art_title, $art_time, $art_date, $art_article, $art_userid) = split (/\|/,$_);
    if ( $art_num eq $form_line && $form_userid eq $art_userid)
    {
        ### update details
        $art_title = $news_title;
        $art_article = $news_article;
    }
    $record = join('|', $art_num, $art_author, $art_title, $art_time, $art_date, $art_article, $art_userid );
    push @updates_file, $record . "\n";
   
}


open (DB,">DB.txt") || die "cant open DB:$!";
print DB @updates_file;
close (DB);

print qq~
$art_num, $art_author, $art_title, $art_time, $art_date, $art_article, $art_userid
<p>
end script\n\n

<form method=POST>
<input type=submit name=qs value=choose>
</form>

~;
}
Avatar of Tintin
Tintin
Lot's there, but I'll give you a few general pointers.

1.  Not using the standard CGI.pm module.  This will shorten your code significantly, and make it much more robust.
2.  Not checking the success of opening files.
3.  No file locking.
4.  Reading files needlessly into memory.
Avatar of KenHeckert
KenHeckert

ASKER

I have no knownledge of the CGI.pm module.  I know that the files are opening, always have... that wasn't the problem.  Why would I need to lock the files?  I don't understand your comment about the memory part.

Keep in mind, I'm an amatuer at best when it comes to perl.

The problem has been fixed, it was:

if ( $art_num eq $form_line && $form_userid eq $art_userid)

That line needed to be:

if ( $form_line eq $art_num && $form_userid eq $art_userid)

Please answer my questions, hopefully this won't crash a system or something.
ASKER CERTIFIED SOLUTION
Avatar of Tintin
Tintin
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of KenHeckert
KenHeckert

ASKER

Thanks TinTin, if I have more questions will you be available?
Avatar of Tintin
Tintin
I check EE everyday