note
cchampion
<p>Not golfing, because you won't get decent code from
golfing horrible code.</p>
<p>I'd rather see this code as a spot-the-mistakes execise.</p>
<p>Thus, here come my changes:</p>
<code>
my @quarters = qw/none first second third fourth/;
sub termgrades {
my ($period, $quarter) = @_;
$sth = $dbh->prepare_cached(
qq{SELECT *
FROM $quarters[$quarter]quarter
WHERE userid = ? AND period = ? AND gradetype = ?}
)
or &error( "2", "$userdata[0] $quarters[$quarter] quarter "
. "grades database search failed: $DBI::errstr" );
$sth->execute( $userdata[0], $period, 2 )
or &error( "2", "$userdata[0] quarter grades "
." database search failed: $DBI::errstr" );
}
</code>
<p><b>update</b><br> Changed the <code>@quarters</code> array
to avoid a minor bug. Thanks to [itub].</p>
<p>However, the first blunder, the one that eventually led
to the current horrible code, is that there are four
database tables instead of one.</p>
<p>Thus the first order of business would be to
merge those four tables into one, adding a "quarter" column.</p>
<p>With that in mind, you could rewrite the above function
as follows:</p>
<code>
sub termgrades {
my ($period, $quarter) = @_;
$sth = $dbh->prepare_cached(
qq{SELECT *
FROM quarters
WHERE userid = ? AND period = ?
AND gradetype = ? and quarter = ?}
)
or &error( "2", "$userdata[0] quarter $quarter grades "
. "database search failed: $DBI::errstr" );
$sth->execute( $userdata[0], $period, 2, $quarter )
or &error( "2", "$userdata[0] quarter grades "
." database search failed: $DBI::errstr" );
return $sth; # perhaps ... see text
}
</code>
<p>That said, this function, in its original form, is completely
useless, because it is executing a SELECT request from a locally
scoped statement handler, and its result would be simply lost.
A returning statement for such a handler is needed, if you want
to pursue any practical purpose.
</p>
<p>What else? Ah, yes, the global <code>@userdata</code> array,
of which we have just to assume it exists and has valid data
in it. It should be passed as an argument as well.</p>
<p>FInally, that naked numeric literal "2" used as an argument
to "execute" and "error" should be turned into a variable, so
that whoever has to maintain the code knows what it is about.</p>
420485
420485