The PHP WTF

There's this nagging voice in the back of my head that says, "don't make fun of the readers! They anger easy and won't come back". However, I couldn't resist reponding to Chris's post about using arrays to create strings because it's faster than concatenation!

So this is today's WTF:

You do realize that cating strings is slower than manipulating arrays, right?

// meh...
/* -->8-- */
// update the area code
$stSQL .= "$areacode=$codeData, ";

// update the phone num
$stSQL .= "$phonenum='$phoneData' ,";
/* -->8-- */

// yay!
/* -->8-- */
// update the area code
$update_array[] = "$areacode=$codeData";

// update the phone num
$update_array[] = "$phonenum='$phoneData'";

$query = "UPDATE tablename SET " . implode(', ', $update_array) ....;
/* -->8-- */

And of course, you should always be checking to see if a variable actually exists, rather than simply assuming it will be there (*cough*$_POST[$areacode]*cough*). I hope this is just an oversight while making an example of poor code.

But from this little shell script we find that string concatenation is more than twice as fast than using arrays.

Run Number #1
1199999 - bigString, Time: 0.5843870639801
1199999 - bigString2, Time: 1.0092749595642
Run Number #2
1199999 - bigString, Time: 0.34683203697205
1199999 - bigString2, Time: 1.0810549259186
Run Number #3
1199999 - bigString, Time: 0.35090112686157
1199999 - bigString2, Time: 1.0080449581146
...

Well if you look at the shell script code you'll see I do 100,000 iterations. But if you only do like 20 iterations using an array is actually faster, take a look:

239 - bigString, Time: 0.0015630722045898
239 - bigString2, Time: 0.0010149478912354

Only after about 150 iterations does string concatenation become faster than using an array. See:

1799 - bigString, Time: 0.0024600028991699
1799 - bigString2, Time: 0.0025348663330078

So yea he's right, but then again, who the hell cares about those thousandths of a second? At least I'll be right when generating SQL for tables with more than a few hundred columns! ;b

/petty.

I like to laugh at clever programmers that outsmart themselves. An anonymous user sent in the following.

/* query_fetch - a very smart function.
  It executes and fetches sql data, returning either a value, an array, 
  or a multidimensional array depending on the query results 
 */
 function query_fetch ($query,$start=1,$max=1000)
   {
   ...
   }

The WTF is what query_fetch() returns. Depending on what comes back from the database it returns a single value (1 row, 1 column), an array (1 row, multiple columns), or a multi-dimensional array (multiple rows and columns). Cool right? A single function that does all that... ohh very smart.

This function is pretty dumb ass because it might return a scalar one time and an array the next. But who the fuck cares that debugging becomes a nightmare, or that I have to write extra code?! It'll make me look smart and nothing like shows smart like adding unnecessary complexity and requirements!

Yea it's a pain in the ass that sometimes a scalar pops out and fucks up the code. The solution? Write a smarter function!

function fixate($item) {
 if (($item)&&(!is_array($item))){ return(array(0=>$item)); }
 return($item);
}

From a WTF to an OMG WTF! Fixer functions are so damn wrong! They don't solve the original problem, they add more complexity and they make the original problem worse. They do make you look smarter though and that's what's important! Look how I solved that data ambiguity problem! Aren't I clever?

/sarcasm.

Fabel sent in today's WTF. It short, and it definately made me go WTF... trying to figure out what it does. After looking at it, and looking at it a little more, it appears to check if $x is even or odd. Here's what Fabel had to say:

Heh this wasnt written by any of us in house it was from someone elses code that got used in a credit card validation routine, i noticed it one day and literally went WTF i've never seen anyone use this method to see if a number is odd or not. I had to do a double take to realize what it was checking for im used to seeing either a modulus to check or just a if($x & 1) for checking an odd number.

if ($x/2 != floor($x/2)) {
  $digit *= 2;
}

Okay before we get deeper into this craziness I would like to remind people that MD5() has been available since php3. Plus MD5 is way more secure than MD4... so I introduce you to today's PHP WTF.

<?php
  function getMd4Pwd($pwd) {
    $pwd = trim($pwd);
    if (strlen($pwd) <= 0)
      return "";
    unset($arrOut);
    $strCmd = "/usr/local/bin/md4sum ".$pwd;
    exec($strCmd,$arrOut);
    return strtoupper($arrOut[0]);
  }
?>

But wait! It gets worse... not only are they not using md5(), they execute a shell script to get an MD4 hash! Really you can't make this stuff up...

And what is /usr/local/bin/md4sum you may ask? Well let me show you...

#!/usr/bin/perl -w
use Digest::MD4;
use Unicode::String qw( utf8 );
Unicode::String->stringify_as( "utf16" );
$u8 = utf8( shift );
print Digest::MD4->hexhash($u8->byteswap), "\n";

So we have a PHP script that calls a Perl script to generate an obsolete, insecure MD4 hash. Not only that but Perl doesn't even have MD4 by default, you have explicitly install it. Um...WTF?!

There is an interesting post on the The Daily WTF about Source Control, or the lack of it.

I wanted to share this. The previous developers didn't use any source control, they simply did the same thing by renaming the old files and added new ones. The file names have been changed to protect the guilty:

This WTF is completely a human problem. Given a bad situation people (especially bad programmers) will find the ways to make it worse. The problem gets worse when dumb developers start using those old libraries! Now your messy directories have turned into software requirements! This may sound retarded but this has happened, and will happen!

Remember source control is to your code as accounting is to your business.

I think inexperienced web programmers all make common DB WTFs when starting out. Jim Grill sent in a prime example from a project that he inherited. I'm sure we've all seen similar code before and we've all said, "wtf?!", if not "ytf?!"

<?php
$query = 'SELECT * FROM sometable';
$result = mysql_query($query,$connection);
$count = mysql_num_rows($result);
?>

It should be obvious what's wrong in the example. To count the number of rows all data is needlessly requested and the rows counted in PHP. As the table grows these three lines will get slower and slower. Most people take data transfer from the DB server for granted. However we always should be as efficient as possible, since little things can quickly multiply into big problems.

The fix is relatively simple:

<?php
$query = 'SELECT COUNT(*) FROM sometable';
$result = mysql_query($query,$connection);
list($count) = mysql_fetch_array($result);
?>

Using count(*) will return just the number of rows. Much more efficient.

Everyday I find code examples that make me go WTF. This is a prime example of one. It is a function that takes an item's id number and generates a left zero padded string with the id number.

I guess nobody told them about the str_pad() function.

function getVanPic($item_id) {
    $pic_path = "/path/to/pic/dir";
    if ($item_id >= 1000000) {
        $strPicName = $item_id;
    }
    else if ($item_id >= 100000) {
        $strPicName = "0".$item_id;
    }
    else if ($item_id >= 10000) {
        $strPicName = "00".$item_id;
    }
    else if ($item_id >= 1000) {
        $strPicName = "000".$item_id;
    }
    else if ($item_id >= 100) {
        $strPicName = "0000".$item_id;
    }
    else if ($item_id >= 10) {
        $strPicName = "00000".$item_id;
    }
    else {
        $strPicName = "000000".$item_id;
    }
    $strPicName1 = "w".$strPicName.".jpg";
    if (file_exists($pic_path."/".$strPicName1)) {
        return "van/".$strPicName1;
    }
    $strPicName1 = "W".$strPicName.".JPG";
    if (file_exists($pic_path."/".$strPicName1)) {
        return "van/".$strPicName1;
    }
    return "";
}

Welcome to The PHP WTF. The idea came from Alex of The Daily WTF. Alex has many examples of VB/Java nightmares and I figured the World needs examples of the bad PHP that people write.

To start things off here's an example from my personal collection. Originally published on my personal blog at http://benzo.tummytoons.com/node/view/66, it demonstrates one of the worse things in PHP, variable variables! Ugh!

---

I have found that explaining bad code to non-programmers is usually followed up by, "isn't it a matter of style?". Programming styles may differ but there is a difference between bad style and bad code.

This is today's example of bad code.

case 2:
  $stSQL = "UPDATE associate set ";
  for ($i = 0; $i < sizeof($arrReferralName); $i++) {
    if (strlen(trim(${"assc_".$arrReferralName[$i]."code"})) > 0)
      $stSQL = $stSQL." assc_".$arrReferralName[$i].
               "code=".${"assc_".$arrReferralName[$i]."code"}.",";
    else
      $stSQL = $stSQL." assc_".$arrReferralName[$i]."code=NULL,";
    $stSQL = $stSQL." assc_".$arrReferralName[$i]."='".${"assc_".$arrReferralName[$i]}."'";
    if ($i < (sizeof($arrReferralName) - 1))
      $stSQL = $stSQL.",";     
  }
  $stSQL = $stSQL." WHERE assc_id=".$ul["assc_id"];
  mysql_db_query($mydatabase,$stSQL);
break;

Other than generating a SQL query, nobody, including the original author can tell what the above code is supposed to do at a glance. The code's purpose has been hidden behind variable variables, a PHP 'feature' that should rarely be used.

A requirement of good code is its purpose is immediately apparent. The example requires a lot of investigation to figure out what it does. Investigation takes a lot of effort, it is frustrating, it is aggrivating and it is a waste of time (and thus money).

This is what I rewrote it to. The functionality has been updated to include better data checking and the structure changed to better show the code's purpose.

case 2:
  // Update the Phone Numbers
  $stSQL = "UPDATE associate set ";
  foreach ($arrReferralName as $refname) { // $arrReferralName from inc_referral.php (in common/)
    // DB Column Names
    $areacode = 'assc_'.$refname.'code';
    $phonenum = 'assc_'.$refname;
    // DB Column Data
    $codeData  = format_phonecode($_POST[$areacode]); 
    $phoneData = format_phonenum($_POST[$phonenum]);
    if ($codeData == '')
      $codeData = 'NULL'; 
    // update the area code
    $stSQL .= "$areacode=$codeData, ";
    // update the phone num
    $stSQL .= "$phonenum='$phoneData' ,";
  }
  // chop the trailing comma
  $stSQL = substr($stSQL,0,-1);
  $stSQL .=" WHERE assc_id=".$ul["assc_id"];
  mysql_db_query($mydatabase,$stSQL);
break;  

The new code is double the length of the original, but it is much simplier. It is now apparent that the code uses POST data to generate a SQL query for updating phone number data. The new code uses the $_POST super-variable, and references the data as elements of the array. $_POST[$varname] is a lot easier to understand than ${"assc_".$arrReferralName[$i]."code"}. Especially since the code is old and written to use automatically registered globals!

Being clever is not always a good thing. There should be a balance between clever code and good code. Good code is considerate to those that come after you. Clever code, like above, leaves a legacy of frustration, resentment and expense in time and money.