if, if, if, if, else, else, else, else...*sob*
Most of the WTF's I post here are submitted from the readers. Today I give you one from the code I have to maintain. In fact, I started The PHP WTF because of this code base. There are 'jewels' like today's example scattered all over the place. Today's example shone so brightly that I had to share it.
Curly braces are optional for simple IF/ELSE statements in PHP but there is a fine line between clear, concise code and a WTF. Today's code is perfectly valid but you'll probably stare at it trying to match the ELSE statements to the IF statements. I think this is what happens when your brain is screwed in backwards and you haven't learned about ELSEIF yet...
<?php
//...
if (intval($rowAgent["assc_homecode"]) == 0) {
if (intval($rowAgent["assc_cellularcode"]) == 0) {
if (intval($rowAgent["assc_pagercode"]) == 0) {
if (strlen($row["list_rphone".$jj]) < 2)
$echoBuffer = $echoBuffer."<BR>\n";
else
$echoBuffer = $echoBuffer."<BR>Phone: ".$row["list_rphone".$jj];
}
else
$echoBuffer = $echoBuffer."<BR>Pager: (".$rowAgent["assc_pagercode"].") ".$rowAgent["assc_pager"]."\n";
}
else
$echoBuffer = $echoBuffer."<BR>Cell: (".$rowAgent["assc_cellularcode"].") ".$rowAgent["assc_cellular"]."\n";
}
else {
$echoBuffer = $echoBuffer."<BR>Phone: (".$rowAgent["assc_homecode"].") ".$rowAgent["assc_home"]."\n";
}
//..
?>
The biggest WTF is that all actual work is done so far away from the original IF statement that it forces you to trace backwards to figure out which IF statement an ELSE goes with. The really poor placement of braces and the minimal indenting doesn't help either!
I took a minute to refactor it. I tried to keep as much as the original conditional logic as possible.
<?php
//..
if (intval($rowAgent["assc_homecode"]) != 0) {
$echoBuffer .= "<BR>Phone: (".$rowAgent["assc_homecode"].") ".$rowAgent["assc_home"]."\n";
} elseif (intval($rowAgent["assc_cellularcode"]) != 0) {
$echoBuffer .= "<BR>Cell: (".$rowAgent["assc_cellularcode"].") ".$rowAgent["assc_cellular"]."\n";
} elseif (intval($rowAgent["assc_pagercode"]) != 0) {
$echoBuffer .= "<BR>Pager: (".$rowAgent["assc_pagercode"].") ".$rowAgent["assc_pager"]."\n";
} elseif (strlen($row["list_rphone".$jj]) >= 2) {
$echoBuffer .= "<BR>Phone: ".$row["list_rphone".$jj];
} else {
$echoBuffer .= "<BR>\n";
}
//..
?>
Note, it is not evident in the code but the columns assc_homecode and assc_home is not actually the home phone numbers. It is the business number. At some point somebody decided to use it to hold business numbers. Sort of makes proper column naming a little pointless doesn't it?
