Posted by Paul Kishimoto on January 11, 2007 at 7:26pm
| Project: | Sympal Theme |
| Version: | 5.x-1.x-dev |
| Component: | Code (PHP) |
| Category: | task |
| Priority: | normal |
| Assigned: | Bèr Kessels |
| Status: | closed (fixed) |
Issue Summary
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:
<?php
if ($tabs != ""):
?><?php
print $tabs
?><?php
endif;
?>or
<?php
if (count($primary_links)):
?><?php
print theme('primary_links', $primary_links);
?><?php
endif;
?>or
<?php
if ($search):
?><?php
print $search;
?>
<?php
endif;
?>All three of these usages could be replaced with empty():
<?php
if (!empty($tabs)):
?><?php
if (!empty($primary_links)):
?><?php
if (!empty($search)):
?>... and give the same behaviour.
Tag consolidation
To further slim down and speed up the code, instead of the following:
<?php
if ($tabs != ""):
?><?php
print $tabs
?><?php
endif;
?>(which causes PHP to process three code blocks), use:
<?php
if (!empty($tabs)):
print $tabs;
endif;
?>or
<?php
if (!empty($tabs)) print $tabs
?>or even
<?php
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!
Comments
#1
Thanks for the information. We should put this in the general code guidelines for Drupal: I have seen a lot of $foo == '' comparisons.
#2
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.
#3
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 :)
#4
Applied.
Setting this back to active so it can serve as a reminder...mark it fixed if you prefer.
#5
Marking as fixed. Even if the performance is not measurable, the code is a lot more consistent and cleaner.
Thanks a million!
#6