Re: Loop Question
by arturo (Vicar) on Mar 21, 2001 at 20:46 UTC
|
There isn't enough context here to give definitive answers to your questions.
As written, it's potentially an infinite loop. The loop will run while the condition $CustomerNumber = $Data{"CustomerNumber"} evaluates as true, which will be the case as long as the thing on the right-hand side of that equals sign evaluates as true (see What is true and false in Perl? for an explanation). Presumably, that should be == instead of = in that line (== is for testing, = is for assignment).
update I thought I'd put this in already, but I guess I hadn't : since it *appears* that the loop should only be executed once (you're not changing anything obviously related to the condition being tested), this should probably be an if rather than a while.
Within the loop, it looks like the code queries a database of some sort, and sets a few variables based on what it finds in the database. For any function you're having trouble understanding, read the output of
perldoc -f [function I don't know]
for more information.
HTH!
Philosophy can be made out of anything. Or less -- Jerry A. Fodor | [reply] [d/l] [select] |
|
|
Actually this loop should execute until it finds another customer number. So by changing the equals sign to == instead of = will prevent the endless loop?
| [reply] |
|
|
That will solve the infinite loop problem, but it won't do what you want it to. The basic problem with this code is that neither $CustomerNumber nor $Data{CustomerNumber} are updated within the loop, so *something* will have to change (as written, it's sort of like asking "so, is this number odd?" about the same number over and over instead of (e.g.) "is 5 odd?" the first time, and then "is 6 odd?" the second time)
You're using SQL, which standardly returns sets of results. The standard Perl DBI interface -- which you are not using here -- would have you do something like this:
my $sth = $db->prepare("SELECT foo, bar FROM Cust_ac_da WHERE Customer
+ID = ? AND #TranType= 'P'")
or die "prepare failed: " . $db->errstr()."\n";
$sth->execute($CustomerNumber);
while (my @stuff = $sth->fetchrow() ) {
# do things with @stuff
}
$sth->finish;
here, what happens is that the SQL statment is prepared, then executed (filling in the customer number where the ? occurs in the prepare); when the statement is executed, what happens is a result set is returned, and in this case each call to $sth->fetchrow() returns the next row in that set until there's nothing left in the set. Since you want to go on as long as you're still getting rows data from the database, you should have the test condition be the data-fetching statement.
Things are complicated by the fact that you appear to be using a non-standard interface to your database (at a guess, the person who wrote it designed their own interface on top of DBI). This makes it much harder to recommend how exactly you should go on!
A word of advice : if you're really new to programming and how control structures work, you should really bone up on the basics before you continue. DBI is getting pretty advanced for someone at your stage.
Philosophy can be made out of anything. Or less -- Jerry A. Fodor | [reply] [d/l] [select] |
|
|
|
|
|
|
(Ovid) Re: Loop Question
by Ovid (Cardinal) on Mar 21, 2001 at 21:13 UTC
|
Couple of quick comments.
As already mentioned, "=" will almost always evaluate as true (however, if $Data{"CustomerNumber"} evaluates as false, then the entire statement will be false).
First, print out the SQL.
my $sql = "SELECT * FROM Cust_act_da WHERE CustomerNumber = $CustomerN
+umber AND #TransType = P";
print $sql;
$db->SQL( $sql );
Once you see the SQL, the error may be obvious. If not, try running the SQL manually and see what the results are.
Also, as a general rule, "select *" is generally not a good way to execute an SQL query (IMHO). It's not as efficient as selecting specifically what you want. Further, if the field names have been changed, this may obscure where your error is coming from.
Another thing that concerns me is that I see you are printing . This suggests that this information is being printed to a Web page. What is the original source of the data? Blindly printing data to a Web page doesn't usually cause problems (though someone could insert a meta refresh tag that redirects your page), but since you don't untaint this data, I am concerned about what else you may be doing with this data that may be dangerous.
Of course, that last comment is totally unrelated to your question, but I always harp about anything that suggests to me that a poor security model is being used. No offense. See perlsec for more information.
Cheers,
Ovid
Join the Perlmonks Setiathome Group or just click on the the link and check out our stats. | [reply] [d/l] |
|
|
When I run just the SQL manually in Access, it works fine. I get the output I want: All "P" transactions for "x" customer.
I am getting this data from a database.
I will try your suggestion to print out the SQL and see what happens from there. Thanks.
| [reply] |
|
|
my $dbh = DBI->connect("dbi:ODBC:$DSN", $USER, $PASSWORD,
{RaiseError => 1})
By doing that, you won't have to test for errors everywhere. It will automatically throw an exception on an error. Also, if you really want to see what DBI is doing under the hood, try the DBI->trace method. This node has a good example of how it works. (That's a shameless plug because it's one of my nodes :)
Just make sure that you set the trace number low, at first. If it's too high, your output file will be flooded with a bunch of information that is probably meaningless.
Cheers,
Ovid
Join the Perlmonks Setiathome Group or just click on the the link and check out our stats. | [reply] [d/l] |
|
|
The SQL printed out fine. It is what I want it to query.
I forgot to take the # sign out of the code when I copied it to the question. But that isn't the problem.
| [reply] |
Re: Loop Question
by dws (Chancellor) on Mar 21, 2001 at 22:04 UTC
|
Are you sure this is the query you're running against Access? Is it possible that you've dropped some quotes along the way? I strongly suspect that
WHERE CustomerNumber = $CustomerNumber
should probably read
WHERE CustomerNumber = '" . $CustomerNumber . "' ...
(though you'll need to protect against someone entering a ' in the CustomerNumber field), and
AND #TransType = P");
should read
AND #TransType = 'P');
And check return codes! $db->Sql() may be leaving valuable information around that you're ignoring.
| [reply] [d/l] [select] |
|
|
" ... WHERE CustomerNumber = '" .
$CustomerNumber . "' ... "
Why isn't that just:
" ... WHERE CustomerNumber = '$CustomerNumber' ... "
--
<http://www.dave.org.uk>
"Perl makes the fun jobs fun
and the boring jobs bearable" - me
| [reply] [d/l] |
|
|
From your suggestion, this is what i printed:
$Sql="SELECT * FROM Cust_act_da WHERE CustomerNumber = $CustomerNumber
+ AND TransType = 'P'";
print $Sql;
$db->SQL( $sql );
and I get this when SQL is printed:
"SELECT * FROM Cust_act_da WHERE CustomerNumber = 13 AND TransType = 'P' "
This is what I have in my code:
while ($CustomerNumber = $Data{"CustomerNumber"}) {
$db->Sql("SELECT * FROM Cust_act_da WHERE CustomerNumber = $C
+ustomerNumber AND TransType = 'P'");
| [reply] [d/l] [select] |
|
|
I jumped the gun on this. If you can guarantee that $CustomerNumber will only hold digits, and CustomerNumber is a numeric field in the database, then don't quote it in the query.
| [reply] [d/l] [select] |
Re: Loop Question
by quietone (Initiate) on Mar 21, 2001 at 23:09 UTC
|
This is what I did. Will this work now?:
$SqlStatement=$db->prepare("SELECT TransDate, TransPmtAmt FROM Cust_ac
+t_da WHERE CustomerNumber = ? AND TransType = 'P'") or die "prepare f
+ailed: " . $db->errstr()."\n";
$SqlStatement->execute($CustomerNumber);
while ( @stuff = $SqlStatement->fetchrow() ) {
# do things with @stuff
$FormatTransDate = substr($TransDate, 0, 10);
$FormatTransPmtAmt = sprintf("\$%.2f", $TransPmtAmt);
print $FormatTransDate;
print " ";
print $FormatTransPmtAmt;
}
$SqlStatement->finish;
| [reply] [d/l] |
|
|
I think you're getting closer
$SqlStatement=$db->prepare("SELECT TransDate, TransPmtAmt FROM Cust_ac
+t_da WHERE CustomerNumber = ? AND TransType = 'P'") or die "prepare f
+a
+iled: " . $db->errstr()."\n";
good. the die is a plus.
$SqlStatement->execute($CustomerNumber);
good. maybe a die here.
while ( @stuff = $SqlStatement->fetchrow() ) {
good. this fixes the seemingly infinite loop that appeared in your first code.
# do things with @stuff
$FormatTransDate = substr($TransDate, 0, 10);
$FormatTransPmtAmt = sprintf("\$%.2f", $TransPmtAmt);
print $FormatTransDate;
print " ";
print $FormatTransPmtAmt;
}
This is a problem, though. You go to all the trouble of preparing, executing and retrieving data, but where do you use it?
See, the row you're currently working on is stored in @stuff, but you never do anything with it. Do you expect $TransDate, $FormatTransDate and $FormatTrnasPmtAmt to hold information from the database? They're not going to. At least, not from the snippet you posted. If they're defined somewhere else in the code, tell us, and say what they're used for. Either assign them values from @stuff or use @stuff directly.
I'm willing to help you, but this is a little much... If you have more questions, please, let me know you looked in DBI's POD, or in perl's reference, or perlvar or somewhere. I don't know what you're doing, or why you're changing things, because the alterations have fixed one thing, but then broken another, or just don't make sense. We can try to make this work, but you gotta reach half-way, OK?
| [reply] [d/l] [select] |
|
|
Okay...i ran this code and i die after the sql statement. Is this because I am not using the DBI interface???
Is there a way to do this without the DBI interface?
I see what you are saying about the @stuff....
The 2 format variables are defined in that loop.
$TransDate and $TransPmtAmt is supposed to be variables used to get data from the columns TransDate and TransPmtAmt in the database.
So in order to get that information I have to do this:
$TransDate=$stuff[1];
$TransPmtAmt=$stuff[2];
$FormatTransDate = substr($TransDate, 0, 10);
$FormatTransPmtAmt = sprintf("\$%.2f", $TransPmtAmt);
print $FormatTransDate;
print " ";
print $FormatTransPmtAmt;
| [reply] [d/l] |