4
\$\begingroup\$

I am currently changing some PHP code. The old code looked like:

<div class="contact<?php echo $this->pageclass_sfx; ?>" itemscope itemtype="https://schema.org/Person">
	<?php if ($tparams->get('show_page_heading')) : ?>
		<h1>
			<?php echo $this->escape($tparams->get('page_heading')); ?>
		</h1>
	<?php endif; ?>

	<?php if ($this->contact->name && $tparams->get('show_name')) : ?>
		<div class="page-header">
			<h2>
				<?php if ($this->item->published == 0) : ?>
					<span class="label label-warning"><?php echo JText::_('JUNPUBLISHED'); ?></span>
				<?php endif; ?>
				<span class="contact-name" itemprop="name"><?php echo $this->contact->name; ?></span>
			</h2>
		</div>
	<?php endif; ?>

	<?php $show_contact_category = $tparams->get('show_contact_category'); ?>

	<?php if ($show_contact_category === 'show_no_link') : ?>
		<h3>
			<span class="contact-category"><?php echo $this->contact->category_title; ?></span>
		</h3>
	<?php elseif ($show_contact_category === 'show_with_link') : ?>
		<?php $contactLink = ContactHelperRoute::getCategoryRoute($this->contact->catid); ?>
		<h3>
			<span class="contact-category"><a href="<?php echo $contactLink; ?>">
				<?php echo $this->escape($this->contact->category_title); ?></a>
			</span>
		</h3>
	<?php endif; ?>

I found this quite hard to follow and decided to try and rewrite the same piece of code. It ended up looking like this:

<?php
echo '<div class="contact'. $this->pageclass_sfx . '" itemscope itemtype="https://schema.org/Person">';

// show/hide heading  
if ($tparams->get('show_page_heading')){
	echo '<h1>'. $this->escape($tparams->get('page_heading')) . '</h1>';
}
	
// show/hide contact name 
if ($this->contact->name && $tparams->get('show_name')){
	echo '<div class="page-header">';
	echo '<h2>';
	if ($this->item->published == 0){
		echo '<span class="label label-warning">' . JText::_('JUNPUBLISHED') . '</span>';
	}
	echo '<span class="contact-name" itemprop="name">'. $this->contact->name . '</span>';
	echo '</h2>';
	echo '</div>';
} ?>

I think the logic is much easier to follow when written like this.

  • I was wondering if anyone had some opinions / remarks / feedback on improvements for the second version of the code.
  • If anyone sees some advantages of the first version over the second I would love to hear what they are.
\$\endgroup\$
2
\$\begingroup\$

The answers you'll get will be highly opinion-based, I guess. So, here a few points in favor of the first type of templating as the approach of inserting short PHP-elements into the template has some advantages.


Easier to read and follow

For a developer, the latter approach might be easier to follow. But often it's important to understand the structure and hierarchy of the markup. Your approach makes it quite hard:

echo '<h1>'. $this->escape($tparams->get('page_heading')) . '</h1>';
echo '<div class="page-header">';
echo '<h2>';
if ($this->item->published == 0){
    echo '<span class="label label-warning">' . JText::_('JUNPUBLISHED') . '</span>';
}
echo '<span class="contact-name" itemprop="name">'. $this->contact->name . '</span>';
echo '</h2>';
echo '</div>';

It looks like everything is on one level and only the span-element is nested. This might be ok for a short passage like we see here. But consider a complex component with lots of nested elements.

Somebody who understands HTML can maintain the template

The first point brings us straight to this: A fronted-developer, who is not familiar with PHP can maintain and update your template. Taking the example from above, the person knows easily where to adjust the code, without breaking anything, if it would be written like this:

<h1><?=$this->escape($tparams->get('page_heading'))?></h1>
<div class="page-header">
  <h2>
    <span class="label label-warning"><?=($this->item->published ? JText::_('JUNPUBLISHED') : '')?></span>
    <span class="contact-name" itemprop="name"><?=$this->contact->name?></span>
  </h2>
</div>

Also removing parent wrappers or inserting new elements is much easier.

Too much logic in the templates

I would say, that with the latter approach one might tend to include too much logic into the template. "Oh, I forgot to load the all products and categories from the database. Well, I'll just include it right here." And yes, this could happen with the first approach as well:

<?php elseif ($show_contact_category === 'show_with_link') : ?>
    <?php $contactLink = ContactHelperRoute::getCategoryRoute($this->contact->catid); ?>

This should happen somewhere in the controller or any other part before the template. However, I find it easier, to keep logic out of the presentation layer, if I'm filling and not creating a template.

Help from the IDE

Your IDE will find problems in the template, like unclosed elements, if you're actually writing HTML instead of creating strings containing HTML.

Switching to a different template engine or even language

At some point you might want to switch from PHP templating to a template engine like Twig or Moustache. It's way easier when you have everything ready for this:

<h1><?=$this->escape($tparams->get('page_heading'))?></h1>

Can easily become this in Twig – as it automatically escapse strings:

<h1>{{ page_heading }}</h1>

Or if you switch to Moustache you aren't even coupled to PHP anymore. You could use your template with a lot of other languages. Or you might even want to include the template in a native APP in the future.

\$\endgroup\$
  • \$\begingroup\$ Some good points there. The code is actually from the Joomla! core. So the argument that someone who just understands html would be able to maintain it, might have been relevant in the choice of their writing style. The main problem seems to be that if statements are spread out over multiple php tags. Which gets especially tedious with embedded if statements. It seems to be a trade-off between logic and 'html accessibility' (for lack of a better word). Any ideas how to get the best of both worlds? \$\endgroup\$ – RMo Dec 18 '17 at 11:37
  • \$\begingroup\$ @RMo as nobody understands why the first one (a widely accepted standard) is hard to follow for you, and what is particularly tedious for you, it's hard to answer anything than "it's ok for everyone else". Nevertheless, you have already given the answer on the "best of the two worlds" - a template engine like Twig \$\endgroup\$ – Your Common Sense Dec 18 '17 at 11:56
  • \$\begingroup\$ what if i want all pages had same header and footer and sidebar ? then in one of pages i want to add css files and change page title , then after one year i want to change sidebar to another thing , if i put html header and body in function and side bar in function and footer in function also , its easy to change all page in one time \$\endgroup\$ – mohade Dec 18 '17 at 12:13
  • \$\begingroup\$ @mohade I'm not sure, whether I understand your point correctly. But when working with PHP templates you should consider using dynamic partials for single components or elements. This way you can compile your template like you want it and add or remove parts from it easily. This is also the way template engines like Twig work. They have partials, includes, template inheritance and other ways to compose and re-use simpler HTML blocks. \$\endgroup\$ – insertusernamehere Dec 18 '17 at 14:56

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.