Coding horrors from the past!

3 05 2006

It’s nice to be back into coding for a while. Of course, one of the drawbacks is coming across code written over half a decade ago that should never have seen the light of day. Take for example this gem I ran across while continuing with my updating Keystone project…

$pq = db_query("select dtable,dcolumn,dflags from dictionary $wclause");
while ($pd = db_fetch_array($pq)) {
$varname = "op_$pd[dcolumn]";
eval("\$oval=\"\$$varname\";");
$varname = "sp_$pd[dcolumn]";
eval("\$sval=\"\$$varname\";");
if (empty($oval)) {
$oval=0;
}
$active = $sval ? "1" : "0";
$subtable = ($pd[dtable] == $proptable) ? '' : $pd[dtable] ;

It’s been said there’s a special hell for people who write code that uses the eval statement in production code. Apparently I’m headed there already. This was a very bad coding decision, but I remember actually writing this particular snippet. It was around 1998, and I was flying to California to talk with the company that would eventually buy Keystone from me. I seem to do some of my best coding work while flying on airplanes, though this sample isn’t exactly a sterling example of it. It did, however, enable one of the cooler features of the product – the ability to, using any of the various data sets, set up a custom browse view based on the structure of the table.

Using ‘eval()’ statements and depending on global variables was NOT the way to implement it though. Now that I’m converting all 12,000 some odd lines of Keystone code over to support running on a system that doesn’t have register_globals enabled, it was time to update this particular code snippet. It took a good 1/2 hour to figure out exactly what it was doing, but once I did that, it was a simple change to:

$pq = db_query("select dtable,dcolumn,dflags from dictionary $wclause");
while ($pd = db_fetch_array($pq)) {
$oval = $_POST["op_" . $pd[dcolumn]];
$sval = $_POST["sp_" . $pd[dcolumn]];
if (empty($oval)) {
$oval=0;
}
$active = $sval ? "1" : "0";
$subtable = ($pd[dtable] == $proptable) ? '' : $pd[dtable] ;
}

Don’t see much of a difference? It’s a big one from a code security and design standpoint. Don’t sweat it too much, it means a lot to me at least.

The conversion is moving along nicely though. I think I can have eval versions ready for folks to test out within a day or three, if I keep this pace up. I’ll be curious to see what sort of response I’ll get on the net to the system. It’s been a while.

Advertisements

Actions

Information

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s




%d bloggers like this: