Skip navigation.

if, if, if, if, else, else, else, else...*sob*

DB Hoopla | Fugly Code

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...

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";
$echoBuffer = $echoBuffer."<BR>Phone: ".$row["list_rphone".$jj];
$echoBuffer = $echoBuffer."<BR>Pager: (".$rowAgent["assc_pagercode"].") ".$rowAgent["assc_pager"]."\n";
$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.

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?

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.

Switch is a little cleaner,

Switch is a little cleaner, but the code still lacks any sense of clarity - ignoring the fact the snippet embeds 'english' into the code, so it adds a future translation nightmare.

switch (true) {
case (intval($rowAgent["assc_homecode"]) != 0):
$echoBuffer .= "Phone: (".$rowAgent["assc_homecode"].") ".$rowAgent["assc_home"]."\n";
break; case (intval($rowAgent["assc_cellularcode"]) != 0):
$echoBuffer .= "Cell: (".$rowAgent["assc_cellularcode"].") ".$rowAgent["assc_cellular"]."\n";
break; ..... default: $echoBuffer .= "\n";}

I'm guessing the intval stuff is really doing !empty(), and perhaps using an associative array with foreach makes it even clearer, and more flexible..

$array = array(
'assc_homecode' => 'Phone', 'assc_home' => 'Home',

foreach($array as $key=>$value) {
if (!$key) {
$echoBuffer .= $value;

if (empty($rowAgent[$value])) {
$echoBuffer .= "<BR>{$value}: ({$rowAgent[$key]})";
}(ps - stick to text comment system, this one is just a pita to use)

I agree with the English em

I agree with the English embedded in there. You should see how the French is done, your eyes will melt.

if ($lang == 'french') {
echo '...';
} else {
echo 'english...';

Doing code in the HTML editor isn't exactly the easiest thing to do. Especially since it converts blank lines to paragraphs. Hopefully in the newer versions those little problems will be fixed.

In the original code it says:

In the original code it says:

if (strlen($row["list_rphone".$jj]) < 2) $echoBuffer = $echoBuffer."
\n"; else $echoBuffer = $echoBuffer."
Phone: ".$row["list_rphone".$jj];

In your new code it says:

} elseif (strlen($row["list_rphone".$jj]) < 2) { $echoBuffer .= "
Phone: ".$row["list_rphone".$jj]; } else { $echoBuffer .= "
\n"; }

Either it was a bug previously, or you just refactored it way too fast ;-)

You should inverse the two last ones resultĀ ;-)

Or am I too sleepy?

Fixed. Nope you are correct!

Fixed. Nope you are correct! I blame it on the perils of code with logic written backwards! :)

Truthfully, following the fir

Truthfully, following the first example was far simpler than the second. But I agree with Alan... I would use switch/case for that many if/elses.

I would have to disagree with

I would have to disagree with you both. Especially when using switch (true) .... I think that's an inappropriate use of switch. Not to mention that the code is more ugly as you add conditions.

I think switch is used to compare a variable to a bunch of simple, constant values in the code. I don't think using switch as a replacement for if/else/elseif is a good or proper use.

This really isn't that bad, b

This really isn't that bad, but your point is good - when this happens over a hundred or more lines, when you have multiple peaks and valleys of ifs, or a really long single peak, matching everything up (and simply indenting properly) is cruel. Most often seen from people who like calling one big DoEverything() function, or purely global code.

The other alternative is running it into a single monster ternary statement. Hey, you're only assigning one variable, why waste all those extra bytes of code?

texas holdem

empirepoker - poker tips, texas holdem | online poker - texas holdem poker, internet poker | poker tournaments - party poker, poker games | poker tournaments - online poker, pacific poker | empirepoker - poker books, poker stars | poker supplies - online poker rooms, online poker | paradise poker - poker stars, poker | poker tips - poker tips, poker rules | party poker - free poker online, world poker tour | party poker - world poker tour, poker online | poker tips - texas hold'em, online poker

world poker tour

online poker - pacific poker, world series of poker | free poker online - pacific poker, poker rooms | empire poker - online poker rooms, texas holdem poker | poker rooms - poker online, partypoker | poker tournaments - poker rooms, poker tables | poker books - empire poker, poker tournaments | pacific poker - online poker, free online poker | wsop - online poker sites, poker online | texas holdem poker - poker tips, world poker tour | poker supplies - texas hold'em, paradise poker | poker rooms - poker tables, empirepoker


empirepoker - texas holdem poker, world poker tour | poker supplies - texas holdem poker, texas hold'em poker | pacific poker - poker, party poker | empirepoker - texas hold'em poker, world poker tour | paradise poker - world series of poker, poker online | internet poker - texas holdem poker, poker books | free online poker - poker tournaments, online poker rooms | online poker rooms - online poker rooms, poker tips | texas holdem poker - poker online, online poker | texas hold'em poker - world series of poker, pacific poker | texas holdem - online poker, texas holdem | pacific poker - poker tables, empirepoker | empire poker - poker tables, free online poker | pacific poker - texas hold'em poker, world series of poker | texas hold'em poker - empire poker, poker tables | poker tips - poker books, poker tips | world series of poker - empirepoker, empire poker | paradise poker - empirepoker, poker books

Post new comment


  • You may post code using <code>...</code> (generic) or <?php ... ?> (highlighted PHP) tags.
  • Allowed HTML tags: <a> <em> <strong> <cite> <code> <ul> <ol> <li> <dl> <dt> <dd> <pre> <p> <br>
  • Web and e-mail addresses are automatically converted into links.