in reply to Cleaning up Code
if (scalar @row1 == "6") { @entry1 = (@required , @row1); $digest1 = sha1_hex(@entry1); $sth = $dbh->prepare($query); $sth->execute($digest1); $data1 = $sth->fetchrow_array(); if ($data1 ne $digest1) { push(@entry1, $digest1); $sth = $dbh->prepare($insert); $sth->execute(@entry1); &create_pdf(@entry1); $msg->attach( Type => 'image/gif', Path => "../pdfs/ir_$digest1.pdf", Filename => "ir_$digest1.pdf", Disposition => 'attachment' ) } else { $dberror .= "The following row you submitted: " . "<br> @entry1 " . "<br>already exsits in the database " . "and will note be re-submitted. <br>" } };
Let's start by identifying bugs.
What fetchrow_array returns in scalar context is undocumented.
$data1 = $sth->fetchrow_array();
is wrong. If you want to the first field, use
($data1) = $sth->fetchrow_array();
Is the contents of $dberror text of HTML? You put both into it. At worst, this could be an opening for a cross-site scripting attack.
All over, you are instructing Perl to ignore prototypes. Why do you use & on your subroutine calls?
I note a distinct lack of my. Use use strict;!
Something tells me you don't use warnings either. ("if ($data1 ne $digest1)" would warn if fetchrow_array returned zero rows.) Use use warnings;!
Why is six in quotes in "if (scalar @row1 == "6")"? You want to compare with the number 6, not the string "6" (which would be converted to a number by ==). The scalar is useless too. == creates a scalar context, so scalar is redundant.
As for your actual question, replace
if (@row1 == 6) { my @entry1 = (@required , @row1); ... }
with
for my $row ( 1 .. $#rows ) { if (@{$rows[$row]} == 6) { my @entry = (@required , @{$rows[$row]}); ... } }
and remove all other mentions of "1".
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re^2: Cleaning up Code
by atmosphere (Novice) on Dec 10, 2008 at 02:44 UTC | |
by ikegami (Patriarch) on Dec 10, 2008 at 03:37 UTC | |
by atmosphere (Novice) on Dec 10, 2008 at 04:38 UTC | |
by ikegami (Patriarch) on Dec 10, 2008 at 06:17 UTC | |
by atmosphere (Novice) on Dec 10, 2008 at 15:12 UTC | |
|