empty()
At http://php.net/empty, the comment by 'inerte is my gmail.com username' states that using empty() is about 20% faster than comparison with '' or "" (zero-length strings). Unlike the following comment's suggested !$var, empty() doesn't generate an E_NOTICE. Page.tpl.php uses a variety of syntax:

if ($tabs != ""):
print $tabs
endif;

or

if (count($primary_links)):
print theme('primary_links', $primary_links);
endif;

or

if ($search):

endif;

All three of these usages could be replaced with empty():

if (!empty($tabs)):

if (!empty($primary_links)):

if (!empty($search)):

... and give the same behaviour.

Tag consolidation
To further slim down and speed up the code, instead of the following:
if ($tabs != ""):
print $tabs
endif;
(which causes PHP to process three code blocks), use:

  if (!empty($tabs)):
    print $tabs;
  endif;

or
if (!empty($tabs)) print $tabs
or even
print empty($tabs) ? null : $tabs

This code is easy to read, involves no shortcuts, and will prevent any E_NOTICE errors. This is pretty much the only tweak I can find in Sympal; it's already a wonderfully pared-down theme!

CommentFileSizeAuthor
#2 sympal_use_empty.patch2.21 KBadrinux

Comments

Bèr Kessels’s picture

Assigned: Unassigned » Bèr Kessels

Thanks for the information. We should put this in the general code guidelines for Drupal: I have seen a lot of $foo == '' comparisons.

adrinux’s picture

Status: Active » Needs review
StatusFileSize
new2.21 KB

This patch converts serveral if statements to using empty. Certainly reduces the code nicely in a couple of places (for tabs and tools), and is I think more readable elsewhere.

I have no benchmarking setup at the moment, so i've no idea what affect this has on speed, but it doesn't *seem* detrimental.

Bèr Kessels’s picture

I won't have much time to test and / or benchmark this, so feel free to commit if you feel confident enough about it.

The code is a lot more consistent and cleaner, with this patch. That alone gets a +1 from me :)

adrinux’s picture

Status: Needs review » Active

Applied.

Setting this back to active so it can serve as a reminder...mark it fixed if you prefer.

Bèr Kessels’s picture

Status: Active » Fixed

Marking as fixed. Even if the performance is not measurable, the code is a lot more consistent and cleaner.

Thanks a million!

Anonymous’s picture

Status: Fixed » Closed (fixed)