in reply to Re^2: Cleaning up Code
in thread Cleaning up Code

I do not understand what you mean by stating I put both into it?

@entry1 contains text and not HTML, right? At the very least, it needs to visit HTML::Entities.

I was under the impression that this was proper usage

It disables prototypes and allows subroutines to be called without parens before they are declared. It was possibly the way to do things in Perl4, but this isn't Perl4.

the primary reason in using "my()" is so that the variables are completely hidden from the outside?

Yes. It limits the scope of the variables. This prevents accidental clobbering of the variables by distant code and it fosters modularity of code. In conjunction with use strict;, typos in variable names are usually caught at compile-time. use strict; helps identify a couple of other type of dangerous code. 

   my @entry = (@required , @{$rows[$row]});
   my $digest = sha1_hex(@entry);
   my $sth = $dbh->prepare($query);
   $sth->execute($digest);
   my ($data) = $sth->fetchrow_array();
   ...

[download]

I actually want it to return with zero rows

Yes, but you don't want it to warn "Use of uninitialized value in string eq" when it does. defined would be one way of avoiding the warning.

So if I read what you stated, I could accomplish the same thing by doing this

Yes. @row1 == 6 checks if @row has 6 elements. An array in scalar context (as imposed by scalar but also by ==) evaluates to the number of elements in the array.

In trying to understand how this statement works

All variables other than @row1 can lose the "1" without problem. The rows are a little bit trickier since we want to switch to using a loop. Unfortunately, you haven't shown us everywhere @rowX is used — in particular, you haven't shown us where they're created — so our help there is limited.

Basically, I suggested that you create an array @rows whose elements are references to arrays (formerly @row1, @row2, etc). This is commonly called an "array of arrays". Basically, Perl's version of a two-dimensional array. There's perllol on the subject, for starters.

Note: I've updated my post to fix a bug in that code.

Replies are listed 'Best First'.
Re^4: Cleaning up Code
by atmosphere (Novice) on Dec 10, 2008 at 04:38 UTC
    Thank you again for your response. I was hoping I could provide this snippet and take what I learn from it and clean up my entire script. However, it appears as if I may need to show you my entire script to ensure I don't shoot myself in the foot later. I appreciate the time you've already spent on this with me but would you be willing, if I show you my entire script, to provide me with some guidance/suggestions to get my script cleaned up?
[reply]
      Sure, I can have a look at it.
[reply]
        What is the best way to get this to you, I'd much rather send it to you than post it on the site as it is a bunch of code. --update-- Actually, I'll be honest.. I didn't want to post it all in fear of being "flamed" of bad usage/practice, but I'll face the humility and post all the code here. Here it is: -- update -- I have updated this code, and it appears as if my only issues are in the code you suggested, any thoughts? 
        #!/usr/bin/perl

use DBI();
use PDF::Create;
use Digest::SHA qw(sha1 sha1_hex sha1_base64);
use MIME::Lite;
use HTML::Template;
use strict;
use warnings;
use CGI qw(:standard);

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

my $template = HTML::Template->new(filename => 'incident.tmpl.html');
my $insert = qq{INSERT INTO incident_report (incident_type, incident_d
+esc, security_impact, reported_by, date_reported, offender_name, offe
+nder_userid, offender_machineid, action_taken, action_date, time_spen
+t, incident_sha) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
} or dienice(dbh->errstr);
my $query = qq{SELECT incident_sha FROM incident_report WHERE incident
+_sha = ?} or dienice(dbh->errstr);

#Sub-routines
sub create_pdf {
my $time = localtime();

my $pdf = new PDF::Create('filename'     => "../pdfs/ir_$_[11].pdf",
                       'Version'      => 1.2,
                       'PageMode'     => 'UseNone',
                       'Author'       => 'author',
                       'Title'        => "ir_$_[11].pdf",
                       'CreationDate' => [ localtime ],
                        );

my $root = $pdf->new_page('MediaBox' => $pdf->get_page_size('A4'));
my $page = $root->new_page;
my $f1 = $pdf->font('Subtype'  => 'Type1',
                 'Encoding' => 'WinAnsiEncoding',
                 'BaseFont' => 'Helvetica');
my $f2 = $pdf->font('Subtype'  => 'Type1',
                 'Encoding' => 'WinAnsiEncoding',
                 'BaseFont' => 'Helvetica-Bold');
my $toc = $pdf->new_outline('Title' => 'Incident Report',
                         'Destination' => $page);
$page->stringc($f2, 30, 306, 800, "Incident Report");
$page->stringc($f1, 10, 306, 782, "created : $time");
$page->stringc($f2, 15, 306, 750, "FORM FOR RECORDING SECURITY INCIDEN
+T RESPONSES");
$page->stringc($f2, 10, 306, 716, "Incident # (online only) = $_[11]")
+;
$page->stringc($f2, 15, 125, 675, "Incident Type");
$page->stringc($f2, 15, 125, 625, "Security Impact");
$page->stringc($f2, 15, 125, 575, "Incident Description");
$page->stringc($f2, 15, 125, 525, "Reported By");
$page->stringc($f2, 15, 125, 475, "Date Reported");
$page->stringc($f2, 15, 125, 425, "Offender's Full Name");
$page->stringc($f2, 15, 125, 375, "Offender's UserID");
$page->stringc($f2, 15, 125, 325, "Offender's MachineID");
$page->stringc($f2, 15, 125, 275, "Action Taken");
$page->stringc($f2, 15, 125, 225, "Date Action Taken");
$page->stringc($f2, 15, 125, 175, "Time Spent");
$page->stringl($f2, 15, 36, 92, "Closure Authorized by:");
$page->stringl($f2, 15, 410, 92, "Date:");
$page->stringl($f1, 15, 220, 675, "$_[0]");
$page->stringl($f1, 9, 220, 575, "$_[1]");
$page->stringl($f1, 15, 220, 625, "$_[2]");
$page->stringl($f1, 15, 220, 525, "$_[3]");
$page->stringl($f1, 15, 220, 475, "$_[4]");
$page->stringl($f1, 15, 220, 425, "$_[5]");
$page->stringl($f1, 15, 220, 375, "$_[6]");
$page->stringl($f1, 15, 220, 325, "$_[7]");
$page->stringl($f1, 9, 220, 275, "$_[8]");
$page->stringl($f1, 15, 220, 225, "$_[9]");
$page->stringl($f1, 15, 220, 175, "$_[10]");
$page->newpath;
$page->rectangle(36,740,530,30);
$page->rectangle(36,660,180,40);
$page->rectangle(216,660,350,40);
$page->rectangle(36,610,180,40);
$page->rectangle(216,610,350,40);
$page->rectangle(36,560,180,40);
$page->rectangle(216,560,350,40);
$page->rectangle(36,510,180,40);
$page->rectangle(216,510,350,40);
$page->rectangle(36,460,180,40);
$page->rectangle(216,460,350,40);
$page->rectangle(36,410,180,40);
$page->rectangle(216,410,350,40);
$page->rectangle(36,360,180,40);
$page->rectangle(216,360,350,40);
$page->rectangle(36,310,180,40);
$page->rectangle(216,310,350,40);
$page->rectangle(36,260,180,40);
$page->rectangle(216,260,350,40);
$page->rectangle(36,210,180,40);
$page->rectangle(216,210,350,40);
$page->rectangle(36,160,180,40);
$page->rectangle(216,160,350,40);
$page->rectangle(75,710,455,20);
$page->line(36,90,530,90);
$page->closestroke;

$pdf->close;
}

sub dienice {
        $template->param(DBERROR => @_);
        print $template->output;
        exit;
}

#Parse Data
my %FORM;
foreach my $field (param()) {
    $FORM{$field} = param($field);
}


#Error Descriptions
my $dberror = "";
my $error = "";
my $itype = "*Please provide a valid incident type.<br>";
my $idesc = "*Please provide an incident description.<br>";
my $impat = "*Please provide a valid security impact.<br>";
my $repby = "*Please provide the full name of the person who reported 
+this incident.<br>";
my $date0 = "*Please provide the date this incident was reported in th
+e proper format.<br>";
my $atake = "*Please provide the action taken to remedy the incident.<
+br>";
my $adate = "*Please provide the date the remedy was applied in the pr
+oper format.<br>";
my $oname = "*Please provide the full name of the offender.<br>";
my $ousid = "*Please provide the offender's usersid.<br>";
my $omaid = "*Please provide the offender's machineid.<br>";
my $spent = "*Please provide the amount of time you spent on this offe
+nder in the proper format<br>";
my $ndata = "*You have failed to fill in all the required fields, plea
+se try again.<br>";

#HTML::Template VAR's
$template->param(ITYPE => $FORM{'incident_type'});
$template->param(SIMPACT => $FORM{'security_impact'});
$template->param(IDESC => $FORM{'incident_description'});
$template->param(REPORTED => $FORM{'reported_by'});
$template->param(DATE0 => $FORM{'date0'});
$template->param(ONAME1 => $FORM{'offender_name1'});
$template->param(ONAME2 => $FORM{'offender_name2'});
$template->param(ONAME3 => $FORM{'offender_name3'});
$template->param(ONAME4 => $FORM{'offender_name4'});
$template->param(ONAME5 => $FORM{'offender_name5'});
$template->param(OUSID1 => $FORM{'offender_userid1'});
$template->param(OUSID2 => $FORM{'offender_userid2'});
$template->param(OUSID3 => $FORM{'offender_userid3'});
$template->param(OUSID4 => $FORM{'offender_userid4'});
$template->param(OUSID5 => $FORM{'offender_userid5'});
$template->param(ATAKEN1 => $FORM{'action_taken1'});
$template->param(ATAKEN2 => $FORM{'action_taken2'});
$template->param(ATAKEN3 => $FORM{'action_taken3'});
$template->param(ATAKEN4 => $FORM{'action_taken4'});
$template->param(ATAKEN5 => $FORM{'action_taken5'});
$template->param(DATE1 => $FORM{'date1'});
$template->param(DATE2 => $FORM{'date2'});
$template->param(DATE3 => $FORM{'date3'});
$template->param(DATE4 => $FORM{'date4'});
$template->param(DATE5 => $FORM{'date5'});
$template->param(TSPENT1 => $FORM{'time_spent1'});
$template->param(TSPENT2 => $FORM{'time_spent2'});
$template->param(TSPENT3 => $FORM{'time_spent3'});
$template->param(TSPENT4 => $FORM{'time_spent4'});
$template->param(TSPENT5 => $FORM{'time_spent5'});
$template->param(OMACH1 => $FORM{'offender_machineid1'});
$template->param(OMACH2 => $FORM{'offender_machineid2'});
$template->param(OMACH3 => $FORM{'offender_machineid3'});
$template->param(OMACH4 => $FORM{'offender_machineid4'});
$template->param(OMACH5 => $FORM{'offender_machineid5'});

#Form Validation
if ($FORM{'incident_type'} =~ /Virus|Spyware|Theft|Destruction|Disclos
+ure|Misuse|Hacking|Error|Failure/)
{       if ($FORM{'security_impact'} =~ /Terminal|Devastating|Critical
+|Controllable|Irritating/)
        {       if ($FORM{'incident_description'} =~ /\w+/)
                {       if ($FORM{'reported_by'} =~ /\w+\s\w+/)
                        {       if ($FORM{'date0'} =~ /\d{4,4}\/\d{2,2
+}\/\d{2,2}/)
                                {       my @required = ($FORM{'inciden
+t_type'}, $FORM{'incident_description'}, $FORM{'security_impact'}, $F
+ORM{'reported_by'}, $FORM{'date0'})}
                                else {$error .= $date0}
                        }else {$error .= $repby}
                }else {$error .= $idesc}
        }else {$error .= $impat}
}else {$error .= $itype}
;

if ($FORM{'offender_name1'} eq "" && $FORM{'offender_name2'} eq "" && 
+$FORM{'offender_name3'} eq "" && $FORM{'offender_name4'} eq "" && $FO
+RM{'offender_name5'} eq "")
{$error .= $oname}

if ($FORM{'offender_name1'} =~ /\w+\s\w+/)
{       if ($FORM{'offender_userid1'} =~ /\w+/)
        {       if ($FORM{'action_taken1'} =~ /\w+/)
                {       if ($FORM{'date1'} =~ /\d{4,4}\/\d{2,2}\/\d{2,
+2}/)
                        {       if ($FORM{'time_spent1'} =~ /\d{2,2}\:
+\d{2,2}/)
                                {       if ($FORM{'offender_machineid1
+'} !~ /\w+/) {
                                                {       $FORM{'offende
+r_machineid1'} = "n/a";
                                                        my @row1 = ($F
+ORM{'offender_name1'}, $FORM{'offender_userid1'}, $FORM{'offender_mac
+hineid1'}, $FORM{'action_taken1'}, $FORM{'date1'}, $FORM{'time_spent1
+'})};
                                                } elsif ($FORM{'offend
+er_machineid1'} =~ /\w+/) {
                                                        my @row1 = ($F
+ORM{'offender_name1'}, $FORM{'offender_userid1'}, $FORM{'offender_mac
+hineid1'}, $FORM{'action_taken1'}, $FORM{'date1'}, $FORM{'time_spent1
+'})};

                                } else {$error .= $spent}
                        } else {$error .= $adate}
                } else {$error .= $atake}
        } else {$error .= $ousid}
}
;

if ($FORM{'offender_name2'} =~ /\w+\s\w+/)
{       if ($FORM{'offender_userid2'} =~ /\w+/)
        {       if ($FORM{'action_taken2'} =~ /\w+/)
                {       if ($FORM{'date2'} =~ /\d{4,4}\/\d{2,2}\/\d{2,
+2}/)
                        {       if ($FORM{'time_spent2'} =~ /\d{2,2}\:
+\d{2,2}/)
                                {       if ($FORM{'offender_machineid2
+'} !~ /\w+/) {
                                                {       $FORM{'offende
+r_machineid2'} = "n/a";
                                                        my @row2 = ($F
+ORM{'offender_name2'}, $FORM{'offender_userid2'}, $FORM{'offender_mac
+hineid2'}, $FORM{'action_taken2'}, $FORM{'date2'}, $FORM{'time_spent2
+'})};
                                                } elsif ($FORM{'offend
+er_machineid2'} =~ /\w+/) {
                                                        my @row2 = ($F
+ORM{'offender_name2'}, $FORM{'offender_userid2'}, $FORM{'offender_mac
+hineid2'}, $FORM{'action_taken2'}, $FORM{'date2'}, $FORM{'time_spent2
+'})};

                                } else {$error .= $spent}
                        } else {$error .= $adate}
                } else {$error .= $atake}
        } else {$error .= $ousid}
}
;

if ($FORM{'offender_name3'} =~ /\w+\s\w+/)
{       if ($FORM{'offender_userid3'} =~ /\w+/)
        {       if ($FORM{'action_taken3'} =~ /\w+/)
                {       if ($FORM{'date3'} =~ /\d{4,4}\/\d{2,2}\/\d{2,
+2}/)
                        {       if ($FORM{'time_spent3'} =~ /\d{2,2}\:
+\d{2,2}/)
                                {       if ($FORM{'offender_machineid3
+'} !~ /\w+/) {
                                                {       $FORM{'offende
+r_machineid3'} = "n/a";
                                                        my @row3 = ($F
+ORM{'offender_name3'}, $FORM{'offender_userid3'}, $FORM{'offender_mac
+hineid3'}, $FORM{'action_taken3'}, $FORM{'date3'}, $FORM{'time_spent3
+'})};
                                                } elsif ($FORM{'offend
+er_machineid3'} =~ /\w+/) {
                                                        my @row3 = ($F
+ORM{'offender_name3'}, $FORM{'offender_userid3'}, $FORM{'offender_mac
+hineid3'}, $FORM{'action_taken3'}, $FORM{'date3'}, $FORM{'time_spent3
+'})};

                                } else {$error .= $spent}
                        } else {$error .= $adate}
                } else {$error .= $atake}
        } else {$error .= $ousid}
}
;

if ($FORM{'offender_name4'} =~ /\w+\s\w+/)
{       if ($FORM{'offender_userid4'} =~ /\w+/)
        {       if ($FORM{'action_taken4'} =~ /\w+/)
                {       if ($FORM{'date4'} =~ /\d{4,4}\/\d{2,2}\/\d{2,
+2}/)
                        {       if ($FORM{'time_spent4'} =~ /\d{2,2}\:
+\d{2,2}/)
                                {       if ($FORM{'offender_machineid4
+'} !~ /\w+/) {
                                                {       $FORM{'offende
+r_machineid4'} = "n/a";
                                                        my @row4 = ($F
+ORM{'offender_name4'}, $FORM{'offender_userid4'}, $FORM{'offender_mac
+hineid4'}, $FORM{'action_taken4'}, $FORM{'date4'}, $FORM{'time_spent4
+'})};
                                                } elsif ($FORM{'offend
+er_machineid4'} =~ /\w+/) {
                                                        my @row4 = ($F
+ORM{'offender_name4'}, $FORM{'offender_userid4'}, $FORM{'offender_mac
+hineid4'}, $FORM{'action_taken4'}, $FORM{'date4'}, $FORM{'time_spent4
+'})};

                                } else {$error .= $spent}
                        } else {$error .= $adate}
                } else {$error .= $atake}
        } else {$error .= $ousid}
}
;

if ($FORM{'offender_name5'} =~ /\w+\s\w+/)
{       if ($FORM{'offender_userid5'} =~ /\w+/)
        {       if ($FORM{'action_taken5'} =~ /\w+/)
                {       if ($FORM{'date5'} =~ /\d{4,4}\/\d{2,2}\/\d{2,
+2}/)
                        {       if ($FORM{'time_spent5'} =~ /\d{2,2}\:
+\d{2,2}/)
                                {       if ($FORM{'offender_machineid5
+'} !~ /\w+/) {
                                                {       $FORM{'offende
+r_machineid5'} = "n/a";
                                                        my @row5 = ($F
+ORM{'offender_name5'}, $FORM{'offender_userid5'}, $FORM{'offender_mac
+hineid5'}, $FORM{'action_taken5'}, $FORM{'date5'}, $FORM{'time_spent5
+'})};
                                                } elsif ($FORM{'offend
+er_machineid5'} =~ /\w+/) {
                                                        my @row5 = ($F
+ORM{'offender_name5'}, $FORM{'offender_userid5'}, $FORM{'offender_mac
+hineid5'}, $FORM{'action_taken5'}, $FORM{'date5'}, $FORM{'time_spent5
+'})};

                                } else {$error .= $spent}
                        } else {$error .= $adate}
                } else {$error .= $atake}
        } else {$error .= $ousid}
}
;

#HTML Feedback
if ($error ne "")
{
$template->param(ERROR => $error);
print $template->output;
}
else
{

#Email Digital Copy to ISO
my $msg = MIME::Lite->new(
       From    => 'pitcher',
       To      => 'catcher',
#       Cc      => '',
       Subject => 'Incident Reports',
       Type    => 'multipart/mixed'
);



#Define Database
my $dbh = DBI->connect("DBI:mysql:database=;host=",
                    "","") or &dienice($DBI::errstr);

#$sth = $dbh->prepare($insert);

#Insert Data
for my $row ( 1 .. $#rows ) {
   if (@{ $rows[$row] } == "6") {
      my @entry = (@required , @row);
      my $digest = sha1_hex(@entry);
      my $sth = $dbh->prepare($query);
      $sth->execute($digest);
      my $data = $sth->fetchrow_array();
      if ($data ne $digest) {
         push(@entry, $digest);
         my $sth = $dbh->prepare($insert);
         $sth->execute(@entry);
         create_pdf(@entry);
         $msg->attach(
            Type     => 'image/gif',
            Path     => "../pdfs/ir_$digest.pdf",
            Filename => "ir_$digest.pdf",
            Disposition => 'attachment')
        }
   } else {
        my $dberror .= "The following row you submitted: <br> @entry <
+br>already exsits in the database and will note be re-submitted. <br>
+"
}
};

$dbh->disconnect;

$msg->send;

if ($dberror ne "") {
        dienice($dberror)
} else {

print <<HTML_SUCCESS

<html><head><title>Confirmation</title>
</head><body>
<h1 align="center">Confirmation</h1>
<p style="text-align: center;">Congratulations you have submitted your
+ information successfully.  Below is the information that has been su
+bmitted.  If the infomation below is not accurate and/or you just rea
+lized the data has an error please notify me.<p>
<TABLE border="1" align="center" summary="">
<CAPTION><EM><BIG><STRONG>Submitted Information</STRONG></BIG></EM></C
+APTION>
<tr>
<th>Incident Type</th><th></b>Brief Description</th><th></b>Security I
+mpact</th><th></b>Reported By</th><th></b>Date Reported</th><th></b>O
+ffender's Full Name</th><th></b>Offender's UserID</th><th></b>Offende
+r's MachineID</th><th></b>Action Taken</th><th></b>Date Action Taken<
+/th><th></b>Time Spent</th><th></b>Incident Number
</tr>
<tr>
<td>$entry1[0]</td><td>$entry1[1]</td><td>$entry1[2]</td><td>$entry1[3
+]</td><td>$entry1[4]</td><td>$entry1[5]</td><td>$entry1[6]</td><td>$e
+ntry1[7]</td><td>$entry1[8]</td><td>$entry1[9]</td><td>$entry1[10]</t
+d><td>$entry1[11]</td>
</tr>
<tr>
<td>$entry2[0]</td><td>$entry2[1]</td><td>$entry2[2]</td><td>$entry2[3
+]</td><td>$entry2[4]</td><td>$entry2[5]</td><td>$entry2[6]</td><td>$e
+ntry2[7]</td><td>$entry2[8]</td><td>$entry2[9]</td><td>$entry2[10]</t
+d><td>$entry2[11]</td>
</body></html>
</tr>
<tr>
<td>$entry3[0]</td><td>$entry3[1]</td><td>$entry3[2]</td><td>$entry3[3
+]</td><td>$entry3[4]</td><td>$entry3[5]</td><td>$entry3[6]</td><td>$e
+ntry3[7]</td><td>$entry3[8]</td><td>$entry3[9]</td><td>$entry3[10]</t
+d><td>$entry3[11]</td>
</tr>
<tr>
<td>$entry4[0]</td><td>$entry4[1]</td><td>$entry4[2]</td><td>$entry4[3
+]</td><td>$entry4[4]</td><td>$entry4[5]</td><td>$entry4[6]</td><td>$e
+ntry4[7]</td><td>$entry4[8]</td><td>$entry4[9]</td><td>$entry4[10]</t
+d><td>$entry4[11]</td>
</tr>
<tr>
<td>$entry5[0]</td><td>$entry5[1]</td><td>$entry5[2]</td><td>$entry5[3
+]</td><td>$entry5[4]</td><td>$entry5[5]</td><td>$entry5[6]</td><td>$e
+ntry5[7]</td><td>$entry5[8]</td><td>$entry5[9]</td><td>$entry5[10]</t
+d><td>$entry5[11]</td>
</tr>
</table>
<p style="text-align: center; font-size: large;"> <font color="red">**
+* PLEASE NOTE *** <br> If you are missing an entire entry please ensu
+re that you have provided the Offender's Full Name for each entry.<br
+>  PLEASE DO NOT REFRESH THIS PAGE, OR GO BACK AND RE-SUBMIT THIS INF
+ORMATION AS THIS WILL RESULT IN DUPLICATE ENTRIES IN THE DATABASE.<p>
</body>
</html>

HTML_SUCCESS
}
};

        [download]
        Again this is my first program so be nice :) Also, the goal of me cleaning up my code is to create a for loop for my data entry, AND data validation.
[reply]
[d/l]

In Section Seekers of Perl Wisdom