1
\$\begingroup\$

This is actually a question that bugged me for 8 years or so, but just swept it under the rug.

I have a project (big, old, zend 1, etc.) that contains a decent (not high but large enough) amount of logic in views. This implies existence of conditionals there. It does not have any template system (like smarty/twig) so php is used for that.

Now... my problem is how to indent (since current indentation just hurts my eyes).

Currently is like this:

  <?php if ($this->product->getType() == Product_Types::BOOK) {
       $bookAuthor = $this->product->getAuthor();
  ?>
  <div><?php echo $bookAuthor; ?></div>
  <?php } ?>

Basically the indentation is correct, but totally useless since I can't easily see that Some output is actually inside the if statement.

Personally I prefer this approach (and used it for years, but now I decided I need a second opinion):

  <?php
     if ($this->product->getType() == Product_Types::BOOK) {
        $bookAuthor = $this->product->getAuthor();
        ?>
           <div>
              <?php echo $bookAuthor; ?>
           </div>
        <?php
     }
  ?>

This is not actually that correct since the <?php ?> tags don't open/close on same space. It basically creates an "pseodo-block" ?> <?php. BUT it helps me see exactly what's where. You can see from first glance that the div belongs in the if... and I think that is actually the purpose of indenting.

How do you guys think is better? (other variants are encouraged)

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review, unfortunately your question at the moment is off-topic for this site. We want real code not example code. You can edit your question with real code. \$\endgroup\$ – Marc-Andre Jan 5 '16 at 15:20
  • \$\begingroup\$ @Marc-Andre Done... although I don't see why that would matter that much considering current question. \$\endgroup\$ – zozo Jan 5 '16 at 15:26
  • \$\begingroup\$ It's not about what matters for the question, it's about what makes a question on-topic for the site. A site being on-topic is good for the question. Prevents it from getting closed :-) \$\endgroup\$ – Mast Jan 5 '16 at 15:31
  • \$\begingroup\$ Indeed, I should have respected that from the beginning, that's why I edited :). Still... thoughts on the actual question? :) \$\endgroup\$ – zozo Jan 5 '16 at 15:33
1
\$\begingroup\$

The second one is definitely better. In the first one, it's really hard to see that the div is actually inside the if.

Personally, I would build the string completely in PHP, and then echo it, instead of switching in and out of the HTML context. This will avoid the opening and closing PHP tags being littered around your code:

$html = "";
if ($this->product->getType() == Product_Types::BOOK) {
    $html += "<div>" . $this->product->getAuthor() . "</div>";
}
echo $html;
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.