Aleksi Kauppila

Posted on Jun 8, 2019

Using isset() and empty() hurts your code

#php

It's a known flaw in PHP that functions in standard library are inconsistent. Some of them have a terrible API that may return anything from object to null and false. Some of them take arguments by reference, like for instance array sorting functions do. In short there's quite a bit features that work in a complicated fashion that makes the code worse.

A part of being a good programmer is to identify features in the language that hurt the code, the software we're building. Once those things are identified, a good programmer will minimize the impact of using those features or completely avoid using them if possible.

So, i've been stumbling lately quite a lot on isset and empty functions. These functions tap really well in to the weakly typed and procedural side of PHP. If you've already been riding the wave of modern PHP for a while, you've learned to hate that side of PHP.

isset

isset function in PHP docs:

isset — Determine if a variable is declared and is different than NULL"

And by example:

php > var_dump(isset($foo));
bool(false)

php > $foo = 'bar';
php > var_dump(isset($foo));
bool(true)

php > $foo = null;
php > var_dump(isset($foo));
bool(false)

php > $list = ['foo' => 'bar'];
php > var_dump(isset($list['baz']));
bool(false)

php > var_dump(isset($list['foo']));
bool(true)

Looking at the definition in PHP docs we can see that the function does two things.

Determine if variable is declared
Determine if variable is different than null

On the example there's three usages that i've witnessed as use cases for isset. Null checking and testing that variable is declared, but also checking if array has a key. First two match the definition in PHP docs and the third is just something developers have started using.

Now, the thing why using isset should be avoided is exactly the thing it promises. It does two things. This causes confusion to the reader.

We don't know if a variable is declared? How did this happen?

Either we are doing something very clever, which as we know is an awful habit, or we are working with a completely procedural legacy system.

If we do know that the variable is declared, then the only reason we'd use isset is for null checking. And for that, i'd recommend explicit null checks. In this code you obviously see easily where $customer came without any mutations. IRL there could be a lot more logic between these statements.

$customer = Customer::find($customerId);
if ($customer === null) {
    // throw
}

An alternative using the same solution that would be even more explicit and clear to the reader would be to null check with instanceof. In this case we know exactly what we're dealing with when we work with $customer.

$customer = Customer::find($customerId);
if (!$customer instanceof Customer) {
    // throw
}

Third situation i've seen, and personally used quite a lot, is checking if an array has a specific key:

if (isset($payload['username'])) {}

This is also another case where there's already a more explicit way to handle the problem.

if (array_key_exists('username', $payload')) {}

I admit, using isset is not the most serious offense. But it still is usually used for either null checking or testing if variable is declared, almost never for both. Being more explicit in these cases will help readers of your code.

empty

empty function in PHP docs:

Returns FALSE if var exists and has a non-empty, non-zero value. Otherwise returns TRUE.

The following values are considered to be empty:
"" (an empty string)
0 (0 as an integer)
0.0 (0 as a float)
"0" (0 as a string)
NULL
FALSE
array() (an empty array)

Wow, so i might be dealing with a string, integer, float, boolean, null or an array. If isset was bad for doing two things, empty is a completely different beast. It does seven things! It answers to seven different questions.

First step to resolve the problem is to use type hints everywhere. Use argument type hints and return type hints in every method and function you write. NOTE: This applies to those developers who work with PHP7 or higher.

Second step is to use more explicit conditionals. This means that the readers of the code don't have to dig deep to boring details when trying to understand what you're trying to accomplish. Using strict comparison (===) is very important.

if (strlen($foo) === 0) {} // Clearly a string
if ($foo === 0) {} // Clearly a number
if ((int) $foo === 0) {} // Clearly expected to be treated as a number
if ($foo === false) {} // Clearly a boolean
if (count($foo) === 0) {} // Clearly an array
if ($foo === null) {} // Clearly null or something useful

As a side note, using return type hints is a very good practice. It will result to more clear interfaces. Once you have a return type hint in place, there's no way you can return multiple different things from your method (assuming you don't use nullable type hints, as you should not).

Are you ready to throw isset() and empty() in to history's trash bin? Tell me what you think!

Top comments (31)

Gerrit Addiks • Jun 9 '19

I cannot say that i agree with the core message of this article. This is written from a mathematical standpoint, when in the majority of cases (at least in PHP) you are expressing semantic facts that are part of a model, not mathematical formulas.

A very important thing to understand when dealing with PHP is that you are not writing kernel-modules or 3D-engines or DBMS's or other performance-driven programs with it. You are using PHP for web-shops, for blogs or for accounting-, advertising- or logistics-web-apps. You are using it for model driven development, for software that tries to emulate a part of "real-life", and that is where semantics are king. The reason why high-level programming was invented was to give us better tools to express the semantics of a model in programming.

In many cases i actually do want to know if something is empty, no matter the technicals. For example: I want to know if the shopping-cart of my user is empty, not if the shopping cart is equal to null (which does not make semantic sense as a sentence). I don't care if at that point my cart is an object, an array or a decimal. I also don't care if the cart is NULL or "0" or an empty array, in all cases the cart can be considered to be empty. Just tell me if the cart is empty or not.
The reason why i often prefer "empty" is that it directly expresses the semantics of the model.

The same argument applies to "isset", it is a semantic operator that tells me if something is there. If it is NULL, it is defined to be "not there", which is exactly what i want to know. In your example-code above, i find the "isset" variant way easier to read.

The PHP expression

if (isset($payload['username'])) ...

directly translates to "If the payload has a username, then ...", which is very easy to read and understand.
In contrast, the PHP expression

if (array_key_exists('username', $payload')) ...

translates to "If the payload has an existing key that is 'username', then ...", which is a lot more convoluted and harder to understand that the "isset" version. But more importantly, it is not a statement that is taken from the model. Who talks like that?!

An even better way to express a check for an empty cart would probably be methods (like "isEmpty") on an shopping-cart object, but in reality (and especially lagacy PHP codebases) you do not always have your semantics expressed in objects, sometimes you have to deal with plain old primitives.

I am onboard with trying to reduce the possible bugs, but this is in my opinion a bad trade-off. You are making the code safer against unexpected (scalar) types, but at the same time you are making it harder to read and to understand by other (mostly junior-) developers because the wording does not relate to the model anymore. That means that when other people try to modify the code (and they will!), they are more likely to be confused by the code and more likely to introduce bugs (compared to when they could just read the code like plain fact-expressing English).
If you want to prevent bugs then the most important thing to do is to make the program easier to understand, problematic corner-cases come second.

Aleksi Kauppila • Jun 10 '19

Thanks for your take Gerrit!

I don't care if at that point my cart is an object, an array or a decimal. I also don't care if the cart is NULL or "0" or an empty array, in all cases the cart can be considered to be empty. Just tell me if the cart is empty or not.

I didn't understand this. Could you provide me some sample code, thanks!

Mark Gullings • Jun 10 '19

On your second paragraph speak for yourself... I develop a factory shop control app mostly on a LAMP stack with ERP integration on an IBMi DB2. Believe me there are plenty of use cases for high DBMS and performance-driven apps in PHP.

As for the sentiment of your comment, yes there are many times isset and empty are useful, and understanding of how they are used is the only requirement for using them. They're my preferred way to test because I know what to expect.

Thomas C. Haflich • Jun 8 '19

The main time I find myself using isset() is for medium-to-big nested arrays, because it evaluates to false instead of throwing an exception if one of the more middling keys doesn't exist.

Eliminates a couple bothersome checks for if ($arr['foo'] && $arr['foo']['bar'] && $arr['foo']['bar']['baz']...), since you can just if (isset($arr['foo']['bar']['baz'][...]). It's not necessary, but convenient for working with more complex data structures.

I can't really think of other times I've needed to use it. Mostly strict equality checks and array_key_exists() work without issue.

Vlastimil Pospichal • Jun 13 '19

Deep structures are unwanted in OOP. Maybe you use it for read a configuration, but read and inject a subtree is more explaining.

Manuel Fernandez • Jun 8 '19

I get the point, but I think isset and empty can be very helpful when you're dealing with an existing codebase, given the fact that you may not understand it fully, or that the code may be doing something different than what you expected.
Great post anyways! I found your point of view very interesting.

Andrii Mishchenko • Jun 9 '19 • Edited

Hi Aleksi,

Good article, thank you. For me it turned something that was more than a "vague feeling" into reasonable position.

There is one thing I'd like to add. In the example:

$customer = Customer::find($customerId);

imo $customer === null is the only reasonable check. instanceof should be used only when we expected find method might return several various instances. In other words, by checking with instanceof we ask "Who are you?", and checking for null - "Do you exist?". And these checks doesn't exclude each other and could be combined when needed.

jmc • Jun 10 '19 • Edited

Just a style thing, but I agree... e.g. $customer instanceof Customer is less explicit than a null test and won't work with things like null object pattern or duck typing.

Aleksi Kauppila • Jun 10 '19

instanceof should be used only when we expected find method might return several various instances.

This actually seems to be case with Laravel, which is what i had in mind when writing this. Bad interfaces and accidental complexity makes you do weird workarounds. :)

David Nash • Jun 8 '19

I've been using PHP for around 15 years. I've never really liked isset() and empty(), mainly because of their names. empty() should be isempty().

Then there are functions like is_int() which use another name style. (I'd prefer they were all named like this).

I STILL have to look up those function names every time. Now I'm not going to bother :-)

And I didn't know about array_key_exists()... Thanks!

Elliot Derhay • Jun 9 '19

Yeah the mixed naming conventions were starting to annoy me too recently. Haven't been using the language nearly as long as you have. But I would hope they would do something like rename all of the runonname functions the other way, alias the old names to the new ones, and then include a deprecation message for the old names and move on. At least that way it would be backwards compatible for however long before they're removed entirely.

Would also prefer renaming some functions like strlen to string_length or at least str_length.

Ben Sinclair • Jul 20 '21

I've been using PHP since version 3 and still have to look up the order of needles and haystacks, or the order in array_map vs array_filter every time.

Devin Handspiker-Wade • Jun 8 '19

The issue is when working with large arrays or being called frequently, array_key_exists is 2x to 10x slower than isset.

Aleksi Kauppila • Jun 9 '19

Interesting, thanks! Do you have a benchmark available somewhere?

Devin Handspiker-Wade • Jun 9 '19 • Edited

It's a simple benchmark with many examples, but to quote one of them provided on the PHP manual:

Benchmark (100000 runs):
array_key_exists() : 205 ms
is_set() : 35ms
isset() || array_key_exists() : 48ms

arturm11 • Feb 11 '20

Already fixed in 7.4

Devin Handspiker-Wade • Feb 11 '20

Interesting! Might need to give 7.4 a look sooner than expected.

Niko Heikkilä • Jun 9 '19

Good post. 👍

Laravel (Eloquent) ships with a nice findOrFail() method for models which throws a ModelNotFoundException when query does not return results. This has been my go-to approach instead of null-checks. I try to communicate with exceptions as much as possible - especially when I'm building a library - helps with debugging both in production and in development.

Aleksi Kauppila • Jun 9 '19

Thanks Niko!

I enjoy using your approach as well! It forces the client to react in case i cannot deliver what i'm asked to. It does a really good job of exposing problems immediately when they happen. I do know that this approach doesn't warrant the admiration of every peer, not sure why though :D Well, hopefully someone will educate me on this one day.

Hans Westerlund • May 14 '20

I know this is a dated article but regarding isset I feel the need to point out the difference between a language construct and a function!

isset isn't a function but a language construct, if you don't know the difference look it up.

TLDR: isset is a lot faster than even built-in functions as is_null. So imho isset should always be preferred cause performance does matter even for websites with moderate loads but certainly for any processing however light.

Takis Antonopoulos • Jan 28 '21 • Edited

isset, empty(), is_int e.t.c are examples of functions of a language with POOR design decisions (at least at the early age of PHP). You have to be precise about what you are offering by a function.

I wish they are removed from PHP and all old snake cased funcions

Artem Goutsoul • Jun 9 '19

Good article! Not totally applicable for legacy code though. Until you have strict typing for properties, parameters and return values, not using isset() or empty() will make the code more verbose and actually less readable and less maintainable.