CODEWITHSUNDEEP
mysqlperl
OrcMan
OrcMan

Reputation: 69

While loop only stores one value from @row=$sth->fetchrow_array() perl

sub completecheckout {
    $cryptedcard = md5_hex($cardnum . $salt);
    $grabcart = qq~select pid from cart where uid='$cookievalue'~;
    $dbh = DBI->connect($connectionInfo, $user, $passwd);
    $sth = $dbh->prepare($grabcart);
    $sth->execute();

    while (@row = $sth->fetchrow_array()) {
        $insert = qq~insert transaction (uid, pid, cctype, ccnum)
                     values ('$cookievalue', '$row[0]', '$cardtype', 
                     '$cryptedcard')~;
        $dbh = DBI->connect($connectionInfo, $user, $passwd);
        $sth = $dbh->prepare($insert);
        $sth->execute();
    }
    $select = qq~select * from registered where id in 
                 (select uid from transaction
                  where uid='$cookievalue')~;
    $dbh = DBI->connect($connectionInfo,$user,$passwd);
    $sth = $dbh->prepare($select);
    $sth->execute();
    @userinfo = $sth->fetchrow_array();

    print header;
    print qq~<html><head><title>YAY</title></head><body><p>CHECK MYSQL<p><p>@row</p></body></html>~;

}

I am trying to parse through the table cart and insert all the items associated with the user into a transaction table when they click the final checkout button. The above code will only insert the last row into the transaction table.

Here is code that inserts more than once, but does not work because $product is empty every other time.

sub completecheckout {
    $cryptedcard = md5_hex($cardnum . $salt);
    $grabcart = qq~select pid from cart where uid='$cookievalue'~;
    $dbh = DBI->connect($connectionInfo,$user,$passwd);
    $sth = $dbh->prepare($grabcart);
    $sth->execute();
    @cart = $sth->fetchrow_array();
    foreach $product (@cart) {
        $insert = qq~insert transaction (uid, pid, cctype, ccnum) 
                      values ('$cookievalue', '$product', '$cardtype',
                              '$cryptedcard')~;
        $dbh = DBI->connect($connectionInfo,$user,$passwd);
        $sth = $dbh->prepare($insert);
        $sth->execute();
    }
    $select = qq~select * from registered where id in
                (select uid from transaction
                 where uid='$cookievalue')~;
    $dbh = DBI->connect($connectionInfo,$user,$passwd);
    $sth = $dbh->prepare($select);
    $sth->execute();
    @userinfo = $sth->fetchrow_array();
    print header;
    print qq~<html><head><title>YAY</title></head><body><p>CHECK MYSQL<p><p>@userinfo</p></body></html>~;

    }

Can anyone explain why this happens? I have been using while loops with fetchrow_array throughout my script to create tables linked to databases.

Upvotes: 1

Views: 1301

Answers (1)

Dave Cross
Dave Cross

Reputation: 69264

Firstly, you need to get into the habit of formatting your code better. It really helps following logic flow if the formatting imitates the logic.

Secondly, please turn on use strict and get used to declaring variables as close to their point of use as possible.

Thirdly, don't use global variables. Your subroutine uses $cardnum, $salt, $cookievalue and several other variables which are (presumably) defined outside of the subroutine. They should all be passed into the subroutine as parameters.

I know from previous conversations that you have no interest in learning Perl, you're just trying to get through a course that your college insists on. So I should make it clear that all of the advice above has nothing to do with Perl. That is all good general advice for any programming language.

Now, the specific problems.

You're creating a new $dbh any time you want to run a database query. Why not just connect once and then reuse that variable. A single $dbh can support multiple queries executing at the same time.

As Matt has pointed out in the comments, you are overwriting $sth. As I said above, a $dbh can support multiple concurrent queries, but each query needs its own statement handle. So you might do something like:

my $dbh = DBI->connect(...);

my $select_sth = $dbh->prepare($select_sql);
$select_sth->execute;

while (my @row = $select_sth->fetchrow_array) {
  my $insert_sth = $dbh->prepare($insert_sql);
  $insert_sth->execute;
}

Notice how I've a) reused the same $dbh and b) declared the $insert_sth within the loop so it's only available for the shorted possible amount of time.

If you were interested in Perl, I'd also show you how to make your code more efficient by using bind points in your SQL and passing extra parameters to execute(). I'd also recommend moving raw HTML out of your program and using a templating engine. But I strongly suspect you wouldn't be interested.

Upvotes: 2

Related Questions