Skip to content

Instantly share code, notes, and snippets.

@gavinblair
Created June 16, 2010 18:33
Show Gist options
  • Save gavinblair/441083 to your computer and use it in GitHub Desktop.
Save gavinblair/441083 to your computer and use it in GitHub Desktop.
Which is more readable? Keep in mind: this is for a view file.
<?php foreach($pages as $page) { ?>
<div id="<?php echo $page['slug']; ?>" <?php echo $user->uid && $_GET['t'] == $page['slug'] ? "class='current setcurrent'" : ""; ?>>
<div class="toolbar">
<h1><?php echo $page['section']; ?></h1>
<?php if(isset($page['parent'])) { ?>
<a href="?t=<?php echo $page['parent']; ?>" class="parent">Back</a>
<?php } else { ?>
<a id="logout" href="/logout" class="button">Logout</a>
<?php } ?>
</div>
<h2><?php echo $page['title']; ?></h2>
<?php if(isset($page['content'])) { ?>
<?php if(is_array($page['content'])) { ?>
<ul>
<?php foreach($page['content'] as $content) { ?>
<?php if(is_array($content)) { ?>
<li <?php echo isset($content['href']) ? "class='arrow'" : ""; ?>>
<?php echo isset($content['href']) ? "<a " : "<span "; ?>
<?php foreach($content as $key => $item) { if($key != "text") { ?>
<?php echo "$key='$item'"; ?>
<?php }} ?>
><?php echo $content['text']; ?>
<?php echo isset($content['href']) ? "</a>" : "</span>"; ?>
</li>
<?php } else { ?>
<li><?php echo $content; ?></li>
<?php } ?>
<?php } ?>
</ul>
<?php } else { ?>
<p><?php echo $page['content']; ?></p>
<?php } ?>
<?php } ?>
<?php if(isset($page['links'])) { ?>
<ul>
<?php foreach($page['links'] as $link) { ?>
<li><a href="?t=<?php echo $link['slug']; ?>"><?php echo $link['title']; ?></a></li>
<?php } ?>
</ul>
<?php } ?>
</div>
<?php } ?>
<?php
foreach($pages as $page) {
echo "<div id='{$page['slug']}' ";
echo $user->uid && $_GET['t'] == $page['slug'] ? "class='current setcurrent'>" : ">";
echo "<div class='toolbar'><h1>{$page['section']}</h1>";
if(isset($page['parent'])) {
echo "<a href='?t={$page['parent']}' class='parent'>Back</a>";
} else {
echo "<a id='logout' href='/logout' class='button'>Logout</a>";
}
echo "</div>";
echo "<h2>{$page['title']}</h2>";
if(isset($page['content'])) {
if(is_array($page['content'])) {
echo "<ul>";
foreach($page['content'] as $content) {
if(is_array($content)) {
echo "<li ";
echo isset($content['href']) ? "class='arrow'><a " : "><span ";
foreach($content as $key => $item) {
if($key != "text") {
echo "$key='$item'";
}
}
echo ">{$content['text']}";
echo isset($content['href']) ? "</a>" : "</span>";
echo "</li>";
} else {
echo "<li>$content</li>";
}
}
echo "</ul>";
} else {
echo "<p>{$page['content']}</p>";
}
}
if(isset($page['links'])) {
echo "<ul>";
foreach($page['links'] as $link) {
echo "<li><a href='?t={$link['slug']}'>{$link['title']}</a></li>";
}
echo "</ul>";
}
echo "</div>";
}
?>
@gavinblair
Copy link
Author

Which is more readable? Keep in mind: this is for a view file.

Indents are nicer in Option 1, but lots of redundant PHP tags, while Option 2 has way fewer characters.

@SeanJA
Copy link

SeanJA commented Jun 16, 2010

Option 2 has fewer characters yes, but you sacrifice any bracket matching / syntax help from your editor... you will inevitably end up with messed up html that will be a pain to fix (especially for someone who is just joining in the project. I find that this really hurts me when I have to use smarty... (question: where does this if statement end? the answer: 4 full pages of text down, I think?)

Personally (you probably already know what is coming) I would make a third option that wraps up some common html elements like links, but if you aren't going to do that, I prefer Option 1 for readability and debugability...

Also I think your usage of the ternary operator, while it does keep the character count down, hurts the readability quite a bit (and you are using ' instead of " for your html attributes! My eyes!!!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment