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

Question

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.

Where to begin? You're clobbering the statement handle ($sth) in the body of the loop, among many more egregious errors. — Matt Jacob
– Matt Jacob, Commented Jul 26, 2017 at 18:44
Can you elaborate?? Not sure how else I can execute the query without having the sth in the loop.. — OrcMan
– OrcMan, Commented Jul 26, 2017 at 18:51
Elaborate... sure. You have while (@row = $sth->fetchrow_array()) { ... }. What do you think happens when you change the loop condition by modifying $sth inside the loop? — Matt Jacob
– Matt Jacob, Commented Jul 26, 2017 at 18:52
Are you implying that you're not formatting your code carefully because you're in a hurry? That makes no sense to me at all. Cutting corners on code formatting will only allow you down. — Dave Cross
– Dave Cross, Commented Jul 27, 2017 at 10:56
And formatting your code carefully has nothing at all to do with your level of knowledge of Perl. Code formatting is a universal programming necessity. — Dave Cross
– Dave Cross, Commented Jul 27, 2017 at 10:58

Dave Cross · Accepted Answer · 2017-07-27 08:58:50Z

2

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.

answered Jul 27, 2017 at 8:58

Dave Cross

69.5k3 gold badges55 silver badges101 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

OrcMan Over a year ago

Thanks David, I am interested in Perl now that I have time to play with it. I may come with more questions and will keep your advice in mind. I had very little time to finish this project as I am doing it alone and it is supposed to be done by 4 people, but I have completed it now. The form of Perl they teach us does not use CGI and so raw HTML is required in our scripts, I am however interested in learning more about Perl with CGI and the next time I have questions I will format them correctly. Thank you for all of your help.

Dave Cross Over a year ago

The form of Perl they teach us does not use CGI and so raw HTML is required in our scripts You are using CGI, you are writing a CGI program. What you're not using is CGI.pm the Perl CGI library. CGI.pm is useful, but you should never use the HTML generation functions that it includes. The only sane way to produce HTML in a Perl CGI program is by using a templating engine like the Template Toolkit.

Dave Cross Over a year ago

Also, as I've mentioned to you before, writing Perl web apps using CGI in 2017 is madness. If you do want to continue your interest in this, then please look at PSGI and the web frameworks built on top of it.

OrcMan Over a year ago

I do use some CGI in my scripts for forms and table building, as well as calling parameters without having to create a loop to parse the POST information from my forms. This is not normally allowed, my teacher made an exception. I have heard Template Toolkit is great to use, however not available on the system we have to use (a school server).

Dave Cross Over a year ago

I do use some CGI in my scripts for forms and table building Yeah. That's the bit we've known to be a terrible idea since the end of the last millennium. And they've been deprecated for a few years. Using CGI.pm to parse parameters is, however, a good idea.

Collectives™ on Stack Overflow

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

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related