27

Over on stackoverflow, I see this issue crop up all the time:

Even Pekka (who offers a lot of solid PHP advice) has bumped against the dreaded E_NOTICE monster and hoped for a better solution than using isset(): isset() and empty() make code ugly

Personally, I use isset() and empty() in many places to manage the flow of my applications. For example:

public function do_something($optional_parameter = NULL) {
    if (!empty($optional_parameter)) {
        // do optional stuff with the contents of $optional_parameter
    }
    // do mandatory stuff
}  

Even a simple snippet like this:

if (!isset($_REQUEST['form_var'])) {
    // something's missing, do something about it.
}

seems very logical to me. It doesn't look like bloat, it looks like stable code. But a lot of developers fire up their applications with E_NOTICE's enabled, discover a lot of frustrating "uninitialized array index" notices, and then grimace at the prospect of checking for defined variables and "littering" their code with isset().

I assume other languages handle things differently. Speaking from experience, JavaScript isn't as polite as PHP. An undefined variable will typically halt the execution of the script. Also, (speaking from inexperience) I'm sure languages like C/C++ would simply refuse to compile.

So, are PHP devs just lazy? (not talking about you, Pekka, I know you were refactoring an old application.) Or do other languages handle undefined variables more gracefully than requiring the programmer to first check if they are defined?

(I know there are other E_NOTICE messages besides undefined variables, but those seem to be the ones that cause the most chagrin)

Addendum
From the answers so far, I'm not the only one who thinks isset() is not code bloat. So, I'm wondering now, are there issues with programmers in other languages that echo this one? Or is this solely a PHP culture issue?

13
  • 4
    Sadly I've seen php devs work with "warnings" turned off as well.
    – Viper_Sb
    Commented Nov 19, 2010 at 17:41
  • Ouch. That is sad.
    – Stephen
    Commented Nov 19, 2010 at 17:49
  • 1
    Not being a PHP user I wonder: why would you need to check for undefined variables? Commented Nov 19, 2010 at 22:35
  • 2
    @Winston: The problem with PHP (in contrast to many other languages) is that PHP allows for many configuration combinations. For instance, the programmer can decide to ignore warnings, notices, and deprecation notifications. More experienced developers will work with error_reporting set to report everything. PHP also has many functions that throw errors and return status codes rather than throw exceptions. One has to be very intimate with the language to be able to code defensively and successfully. Many other languages don't allow the choice. Strict is usually the default and only option. Commented Dec 18, 2010 at 4:59
  • 1
    I dislike isset() because of the verbosity it adds. A better form of @ would be a nice alternative.
    – Kzqai
    Commented Oct 15, 2011 at 6:41

9 Answers 9

34

I code to E_STRICT and nothing else.

Using empty and isset checks does not make your code ugly, it makes your code more verbose. In my mind what is the absolute worst thing that can happen from using them? I type a few more characters.

Verses the consequences of not using them, at the very least warnings.

5
  • 7
    My development environment is also E_STRICT. I suppress all errors in production, but that's just to ensure nothing slipped past me in development.
    – Stephen
    Commented Nov 19, 2010 at 16:07
  • 2
    +1 Josh, I'm the same way. When you program in E_STRICT, you know that you're never going to have surprises in your code. Only way to fly, imo. Commented Nov 19, 2010 at 16:26
  • 3
    @Stephen: Always turn off errors for production. That's just a safety net though. Development should (IMO) always be E_STRICT. Write code and resolve all warnings and errors.
    – Josh K
    Commented Nov 19, 2010 at 16:46
  • are you parroting me? :)
    – Stephen
    Commented Nov 19, 2010 at 17:15
  • @Stephen: I'm agreeing. :)
    – Josh K
    Commented Nov 19, 2010 at 19:49
17

I think the notices on unknown elements are a design mistake in PHP. I'm not sure it's possible to fix the mistake now, but it produces a lot of boilerplate code like if(isset($foo['abc']) && $foo['abc'] == '123') - this code should not have isset, since the intent is to check if there's '123' in certain place of $foo and if there's nothing there it's definitely not '123'. The only reason you have to write twice as much code there is because of this unfortunate design mistake in PHP. And notices are very expensive in PHP, unfortunately, so just disabling them is not an option for the code where performance is a concern.

So yes, it does make the code ugly IMHO and it does annoy me. And it's not because of lack of experience - I use PHP since 1998 and I remember when there was .php3 extension and it meant "it's not PHP 2". Maybe I am lazy :) But laziness - at least certain type of it - is a virtue for a programmer.

On the other hand, valid use of isset and empty - just like ones in the original post - are fine. I just think PHP is over-zealous about warnings in places where isset/empty are not really needed.

3
  • 3
    Right. It's a matter of verbosity. $eg = isset($_GET['eg'])? $_GET['eg'] : null; is ridiculously verbose. I really wish there were another operator that wasn't "error supression" per se to stand in for the short meaning of $eg = @$_GET['eg']; for "accept-the-non-existence-of-this-array-key-as-a-null". I tend to use a short function, these days.
    – Kzqai
    Commented Oct 15, 2011 at 6:39
  • 7
    Agreed. 4 years later, enter the Null Coalesce Operator :wiki.php.net/rfc/isset_ternary giving us $eg = $_GET['eg'] ?? null;
    – Steve
    Commented Oct 10, 2014 at 11:58
  • Or just do it now with the error suppression operator, e.g. $eg = @$_GET['eg'] ?: null;. I don't generally use it, but in this case we are explicitly expecting that error and deciding to ignore it. It's one character longer than the null coalesce and it's not as good (it suppresses all errors, and there might an ArrayAccess doing funky stuff there, not just an array), but in general it does the job.
    – El Yobo
    Commented Sep 23, 2015 at 13:27
12

I believe PHP, as a free language, that is interpreted and used on the web, has a very high proportion of non-professional, untrained coders who are not fully aware of why they should code defensively and just see the warnings as another unnecessary error.

I hear these points from junior developers and self-taught script-coders all the time:

  • Why initialise a variable if it will come in to existence anyway?
  • Why check if something exists before we use it, undefined equates to false anyway?
  • If I prefix with @ it fixes my code, why complicate things?

If they have never experienced a strongly-typed language, or experienced the pitfalls of undeclared/uninstantiated variables, then they take some convincing. I find they usually give in once they have experienced the pleasure of debugging code for an hour or so to find it was a typo in a variable name causing the trouble.

The other strong factor is PHP's use in the web industry, which tends to be far more concerned with throughput than safety and code quality.

4
  • 10
    "I believe PHP [...] has a very high proportion of non-professional, untrained coders" As a dedicated PHP dev, I cannot agree with you more. +1
    – Stephen
    Commented Nov 19, 2010 at 21:55
  • @Stephen Well I code mainly in PHP and jQuery lately, though I do have formal education, but the code that you see as a professional that you become responsible before... it defies belief sometimes. ;-)
    – Orbling
    Commented Nov 19, 2010 at 22:46
  • 1
    I think that's a real problem with PHP. Aside of some quirks in the language (which they're slowly solving in each new version) there's an air of amateurism around it. I can imagine being a professional PHP dev can be frustrating sometimes, because you're being compared to 'the boss's nephew from around the corner'. Commented Nov 22, 2010 at 14:52
  • 1
    @Erik PHP was a fairly poor relative of Perl ten years ago, it is now a fairly reasonable interpreted language, particularly for web usage. There is a reason companies like Facebook use it. But as it is available everywhere, the amateur set use it extensively and this tarnishes the reputation.
    – Orbling
    Commented Nov 22, 2010 at 15:27
5

Yes, they are lazy. A lot of them anyway...

Unfortunately the mentality of many PHP coders is "There is no point coding defensively if you can get the same end result quicker by relying on the language to handle errors caused by missing variables etc etc." I know, I've worked with several of them.

They tend to also be the ones who mysteriously head for an early lunch when their lack of correct error handling and reporting takes out the live servers for several hours...

3
  • 5
    -1 for the subjective opinion on PHP coders.
    – Josh K
    Commented Nov 20, 2010 at 18:39
  • 4
    @Josh, having been a Senior PHP coder for 4 1/2 years and coded PHP on and off since 1999, I'm afraid I am somewhat subjective. I should have qualified with "On the other hand, the more experienced PHP coders aren't lazy and do things correctly"...
    – Gruffputs
    Commented Nov 25, 2010 at 13:04
  • 1
    That you should have.
    – Josh K
    Commented Nov 25, 2010 at 14:06
5

I try to avoid the use of isset() and empty() by avoiding the use of arrays as data transfer objects. Create a class that can be configured to accept a limited set of properties with sane defaults and validate input to those properties. It can even implement the ArrayAccess interface so you can use it like an array. This also allows you to use type hinting in your method signatures to catch errors when someone attempts to pass the wrong type of entity to your method.

1
  • A very interesting approach.
    – Toby
    Commented Nov 26, 2010 at 14:12
2

One of the questioned questions is mine, so let me reiterate.

Many developers mistake the presence of lot of isset() as quality sign. It gives an appearance of reliability and some people think of it even as security feature.

But you have to take into account that PHP is not a compiled language. It's a scripting language with a dynamic type system. E_NOTICE errors are only errors by name. And if lots of isset are used with the sole intention of suppressing the notices, then you are actually just coding against the language.

So, are PHP devs just lazy?

That is indeed the case if you see an abundance of notices and warnings thrown. Lot's of newbies don't care about notices, and it's usually the result of a completely disabled error_reporting(0).

You are however mistaken that other developers didn't recognize E_NOTICEs just because they weren't @ or isset-suppressed. At least that was my intention behind the retaining notices question. They aren't something to get rid of, but occasionally important debug info.

Like all generalizations, the inconsiderate use of isset doesn't lead to optimal code. It's important to differentiate where isset and empty are necessary and where they are syntactic salt.

Or is this solely a PHP culture issue?

No, undefined variable "errors" are not a PHP problem alone. Bash, TCL and Perl or JavaScript allow the use of undefined variables. This immanent language feature isn't seen as defect there however. There are similar language constructs to check for undefined values. However they aren't as constantly used as in PHP, due to undef values not being mischaracterized as "error".

1

Interesting. I hardly ever use isset(). My PHP code is rarely in a state where I dont know if a variable has been set in the first place. Every GET or POST variable used is accessed via a function that will provide it a default if it doesnt exist. Default values in function calls are typically explicitly set to 0 or empty strings.

1
  • +1. Passing to a function or class method is a nice solution. I actually DO do this, but in a pinch my above examples still look clean and un-bloated to me.
    – Stephen
    Commented Nov 19, 2010 at 22:07
0

I use isset and empty plenty. But mostly it's places that you mention, like $_REQUEST processing, in case anyone's been messing with the parameters. In cases where I have control over all the variables flying around, I find that I usually don't need them.

0

I dont like using isset, but if there is masses of code done by another, then it can be a saving grace. I wrote the wee code below to help with this problem, instead of using isset() isseter($a,$b) will return $b if $a is not defined or empty, or the function returns a null. Any improvements, welcome:

//returns var or NULL (if undefined)
//$reserve optional second value is returned as $default if undefined
function isseter(&$default,&$reserve=NULL)
{
$default = isset($default) ? $default : NULL;
$reserve = isset($reserve) ? $reserve : NULL;
if ((!$default) && ($reserve)) $default=$reserve;
return $default;
}

Not the answer you're looking for? Browse other questions tagged or ask your own question.