14

As its file extension suggests a .phtml file allows PHP code to be mixed with HTML. However, the fact that you can should not be seen as a license to go wild.

Why do we still see so many .phtml files riddled with lots of PHP? And what is a good approach to reduce the amount of PHP in a .phtml file?

10

Indeed, the less PHP in your .phtml the better, because:

  1. the mix of the PHP and HTML is much harder to decipher than each of them individually, especially for those comfortable with only one of them (e.g. front-end designers)
  2. it makes logical sense to place interaction with server code in the Block, away from what is to be presented in the browser -- this is the old "separation of concerns” mantra.

The Magento core file /app/design/frontend/base/default/template/catalog/product/price.phtml is a painful case in point. This HTML “presentation” code displays a price. It is 471 lines long! Mostly because of PHP logic.

To make your .phtml leaner and cleaner:

  1. avoid unnecessary sequences of <?php … ?> , bundle them together in chunks with a single <?php … ?>

  2. push as much PHP as you can into the Block, rather than the .phtml

  3. to help with the above, in the Block make use of assign(‘myvar’, [expression]) to create $variables that can be referred to without $this->... in the .phtml, so you can have really concise <?php echo $myvar; ?>

  4. wish for Magento to adopt Twig in the future for an even cleaner look

Let's apply the above on a snippet from the original code of the example given above: /app/design/frontend/base/default/template/catalog/product/price.phtml

<?php if ($this->getDisplayMinimalPrice() && $_minimalPriceValue && $_minimalPriceValue < $_product->getFinalPrice()): ?>

    <?php $_minimalPriceDisplayValue = $_minimalPrice; ?>
    <?php if ($_weeeTaxAmount && $_weeeHelper->typeOfDisplay($_product, array(0, 1, 4))): ?>
        <?php $_minimalPriceDisplayValue = $_minimalPrice+$_weeeTaxAmount; ?>
    <?php endif; ?>
    ….
             <?php echo $_coreHelper->currencyByStore($_minimalPriceDisplayValue, $_storeId, true, false) ?>
  1. First step: remove the repetition of <?php … ?> to arrive at something like this:

    if ($this->getDisplayMinimalPrice() && $_minimalPriceValue && $_minimalPriceValue < $_product->getFinalPrice()) { $_minimalPriceDisplayValue = $_minimalPrice; if ($_weeeTaxAmount && $_weeeHelper->typeOfDisplay($_product, array(0, 1, 4))) { $_minimalPriceDisplayValue = $_minimalPrice+$_weeeTaxAmount; } … echo $_coreHelper->currencyByStore($_minimalPriceDisplayValue, $_storeId, true, false) ?>

The above puts all PHP in a single blob of code.

2 + 3. Evolving into something better still, move this code into its block:

protected function _prepareLayout() {
    $this->assign(‘minPrice’, $this->calculateMinPrice(…));
}

protected function calculateMinPrice(…) {
    if ($this->getDisplayMinimalPrice() && $_minimalPriceValue && $_minimalPriceValue < $_product->getFinalPrice()) {
       // etc...
    }
}

Note the use of the _prepareLayout() and the assign() functions for this.

Now that convoluted section of the .phtml can be reduced to just this simple line:

<?php echo $minPrice; ?>

I think we can all live with that!

5

Nice writeup, @fris, I agree in almost all points.

The main takeaway is to move all logic into the block class and make the template as "stupid" as possible.

I actually prefer method calls in the template over variables that have been "assigned" because I don't want to loose IDE code completion and navigation features. "assign" look more concise in the template but is way too much magic for my taste, making it even worse than the magic getters and setters.

3
  • Appreciate your comment @fschmengler. Yes it is a little magic, but that's the case with all conventions, at first. Using $this inside a .phtml file certainly looked like magic to me the first time I saw it. Now I understand it and it's fine. It's a matter of learning the patterns & conventions. Code-completion is important. However is it a fair call to place pragmatism stemming from tools that are not sophisticated enough over an architectural programming decision?
    – fris
    Aug 22 '15 at 7:12
  • Using as little magic as possible is an architectural decision. If you need additional tools to work with a code base that's a bad sign... To be fair, Magento did not make this decision, but we can strive to make the best of it. Aug 22 '15 at 7:59
  • cool writeup fris. I agree with every point except assign part. the reason is because it become too difficult to find those magic variables for an another developer who is going through it. i think we should avoid that. Aug 22 '15 at 13:16

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.