Log in

View Full Version : Striving for Elegance


marain
10-01-2012, 09:27 PM
Elegance, I suppose, can be subjective. I view




<?php

if ( $here === 'arson' )
echo '<body class = "arson">';

elseif ( $here === 'badcheck' )
echo '<body class = "badcheck">';

elseif ( $here === 'lewdness' )
echo '<body class = "lewdness">;

elseif ( $here === 'posswpn' )
echo '<body class = "posswpn">;

else echo '<body>';

?>




as less elegant than...




<?php

switch ($here) {

case 'arson':
echo '<body class = "arson">';
break;

case 'badcheck':
echo '<body class = "badcheck">';
breaK;

case 'lewdness':
echo '<body class = "lewdness">';
break;

case 'posswpn';
echo '<body class = "posswpn">';
break;

default:
echo '<body>';
}

?>



...even though I'm not sure I can verbalize why. Whether you agree or not, I suspect there are coding methods that improve on both of these snippets, with regard to elegance. I would welcome thoughts and suggestions.

A.
djr33
10-02-2012, 12:27 AM
What are your goals, and what are the requirements?

Personally I find switch statements to be somewhat annoying because they have odd syntax and take up a lot of space. However, you're right that they're a little more elegant than a long string of ifs. But ifs are more versatile than just checking the values on one variable, so I much more often find myself ignoring the option of switch statements.

I think the reason you find it more "elegant" is because it's one statement rather than a but of unrelated ifs. Of course your use of elseif actually does make it tied together in a way too, so that's not too bad. (Technically all you need there are "if" statements because the variable will only ever have one value, so no more than one can be true. But doesn't quite work out with the else, though, so what you have is fine and perhaps slightly more efficient rather than comparing all of the values each time.)

As a general rule, writing clean/clear code is important, even if just to stay organized.


Now... on to the details here. You're mixing logic and content. You should run your PHP decisions at the top of the script then use templating techniques to output all of that later; in other words, you should avoid mixing HTML in the middle of your PHP stuff like that.

Or, if you need to do it, try to limit the PHP.

Here's a simpler way to rewrite your code. I don't know if your situation will allow for this to work, but if it does this is the best way probably:

<?php
if (in_array($here,array('arson','badcheck','lewdness','posswpn'))) {
echo '<body class = "'.$here.'">';
}
else {
echo '<body>';
}
?>

By the way, I always use {} with if. Even if it's not needed (if there's just one line) I strongly prefer it for clarity and to visually offset the chunks.