Monday, October 19, 2009

Coding styles that make me twitch, part 5

Let us say that we have some code using DBI - old-fashioned, but it still works, kindof.

How would you like to see the following in a 6000 line CGI script you're supposed to debug?

my $sth=&query("SELECT id FROM invoices WHERE invoiced='N'");
my $invoiced=$sth->rows;

my $sth2=&query("SELECT count(*) nusers FROM users \
LEFT JOIN people ... # long SQL statement
my $row2 = $sth2->fetchrow_hashref;
my $nusers = $row2->{nusers};

my $sth3=&query("SELECT count(*) npeople FROM users \
LEFT JOIN people ... # long SQL statement
my $row3 = $sth3->fetchrow_hashref;
my $npeople = $row3->{npeople};

my $sth4=...
And then, after carefully checking scope, you discover that the variables $sth2, $row2, $sth3, $row3, $sth4, $row4 etc. are not used anywhere else within the same scope.

Would you develop a tick?

I did, and the tick didn't lessen in strength when I discovered unsafe variable interpolation as well as a sub query that used $dbh->prepare($qtext) but didn't allow passing of usable parameters, such as, you know, bind variables.

I've started on "refactoring" the real life code this example is based on, but I get depressed from the sheer amount of work in fixing many problems at once. Maybe I should pick up drunken coding to become less perfectionist.


derby said...

and was $dbh a global? Or did it connect_cached each time?

bakkushan said...

$dbh is a global in an external package.

connect_cached was not used.