The PHP WTF

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.