Currently our coding conventions have major prohibitions on the use of isset() and empty(). I can understand isset(), because sometimes it makes more sense logically to have boolean casting or strict type checking, but in the case of empty(), it's significantly faster than boolean casting and is more readable.
If you're checking for *whether an array is empty*, which is more intuitive:
if( !$array ) // If the boolean value of the array is false if( empty( $array ) ) // If the array is empty
Functionally, they're almost equivalent, as any value that evaluates as false will also be treated as empty. The sole difference is 1) empty() first checks if the variable is set and 2) empty() is faster.
I understand that there is a slight risk here due to empty()'s error suppression, but in many cases it's obvious that a variable exists and it's not necessary to sacrifice the logical readability of the code just because a developer might make a typo and not realize it even though the same variable is on the line above.
*--* *Tyler Romeo* Stevens Institute of Technology, Class of 2015 Major in Computer Science www.whizkidztech.com | tylerromeo@gmail.com
---------- Forwarded message ---------- From: Brad Jorsch bjorsch@wikimedia.org Date: Tue, Feb 5, 2013 at 5:43 PM Subject: Re: [Gerrit] API PageSet allows generator for non-query modules - change (mediawiki/core[master]) To: tylerromeo@gmail.com
On Tue, Feb 5, 2013 at 5:03 PM, Parent5446 (Code Review) gerrit@wikimedia.org wrote:
Yeah, I see the page, but it doesn't tell me anything interesting. The
only disadvantage of using empty() is for the off chance that you mispell a variable name and the error is hidden, but it's obviously not the case here. And in terms of readability, it makes a lot more sense to have if( empty())
Then perhaps you should raise the question of changing the coding standards on https://www.mediawiki.org/wiki/Manual_talk:Coding_conventions/PHP
Until then, let's not use empty() unless we explicitly want the "isset" check.