if($var)... vs if(isset($var))... Should I care?

WallabyKid

Is it really the conventional wisdom that every variable (index) should be checked against
isset or empty to be sure it has been defined before either using or ignoring it?

Where...

$itemId = $_SESSION['itemId'];

becomes...

if(isset($_SESSION['itemId'])) $itemId = $_SESSION['itemId'];

e.g. twice the code??? Really???
Or Consider...

<?php
	if($var1) $var2='somevalue';
?>
<input type="text" value="<?php echo $var2; ?>" />

which becomes

<?php
	if(isset($var1)) $var2='somevalue';
?>
<input type="text" value="<?php if(isset($var2)) echo $var2; ?>" />

In a large project this could really add up to what appears to me a substantial volume of otherwise superfluous code just to avoid Notice Level Errors?

Can the resulting Notice Errors add significantly to the server load for a given site?

bradgrafelman

WallabyKid;10976291 wrote:
Is it really the conventional wisdom that every variable (index) should be checked against
isset or empty to be sure it has been defined before either using or ignoring it?

Of course not - just those that may or may not exist.

WallabyKid;10976291 wrote:
Where...
$itemId = $_SESSION['itemId'];
becomes...
if(isset($_SESSION['itemId'])) $itemId = $_SESSION['itemId'];

I would much prefer something like:

$itemId = (isset($_SESSION['itemId']) ? (int)$_SESSION['itemId'] : NULL);

since that a) allows me to set a default value, b) allows me to do some basic sanitization/data casting, and c) does (a) and (b) while remaining compact.

WallabyKid;10976291 wrote:
e.g. twice the code??? Really???

Yep. Really. If you don't like that, try skipping such checks in C/C++ code and watch the 'segmentation faults' take over your console. 🙂

WallabyKid;10976291 wrote:

Or Consider...

<?php
	if($var1) $var2='somevalue';
?>
<input type="text" value="<?php echo $var2; ?>" />

which becomes

<?php
	if(isset($var1)) $var2='somevalue';
?>
<input type="text" value="<?php if(isset($var2)) echo $var2; ?>" />

Well now you're just being silly. Why call isset() two times? Just call it once and, if the external variable isn't defined, simply set a default value instead (eliminating the need for a 2nd, 3rd, ... nth call to isset()).

WallabyKid;10976291 wrote:
In a large project this could really add up to what appears to me a substantial volume of otherwise superfluous code just to avoid Notice Level Errors?

Well then there's your problem. Your goal shouldn't be to avoid the error messages themselves, it should be to avoid the actual errors.

Otherwise, yes, this really could (and often does) add up to a lot of code. Welcome to the world of computer programming. 🙂

WallabyKid;10976291 wrote:
Can the resulting Notice Errors add significantly to the server load for a given site?

You mean the actual error logging mechanism? I suppose there is technically some overhead (yet another justification for what you call "superfluous" code), but I doubt it's going to be anywhere near as significant as the primary bottleneck on your site (whatever that may be).

WallabyKid

Well.... color me silly.
Oh... wait, you already did😉

Thanks for the feedback.

WallabyKid

seeing as this is php (e.g. NOT C/C++) and given the assumption of little anticipated discernible impact on performance due to the additional overhead of otherwise suppressed Notices... why should I care?

No seriously... I want to understand why it is important that PHP should spit an error
at

if($var) do_something();

when it seems to me that the isset in that context is tantamount to implicit, and that php itself does in fact treat it as such.

Please, why is it important (best practice)?
Examples....?
Thanks

bradgrafelman

Why should you care about any error? In general, because it's an error; the developers made it that way for a reason.

Why shouldn't PHP warn you about something that could have otherwise brought a program to a crashing halt? Why should it quietly ignore the fact that you're attempting to use variables that don't exist? What if you made a simple typo, e.g. $usrename instead of $username - would you rather see an error message about this undefined variable or silence as PHP quietly assumes you just forgot to define variable $usrename and continues on.

There are probably more reasons - these are just some example situations I came up with off the cuff.

WallabyKid

Why should it quietly ignore the fact that you're attempting to use variables that don't exist? What if you made a simple typo, e.g. $usrename instead of $username

I don't really know. Maybe because -- in general -- the developers provided granular error reporting for a similar reason, e.g. affording you the option of regarding certain errors as important, and others not so much (optionally ignored) -- so facilitating streamlined code that would bring programs (written in lessor languages) to a crashing halt, while maintaining the wherewithal to debug for something like a simple typo should the need arise?

Otherwise, what would be the point of providing granular error reporting? Because the developers made it that way for a reason?

No disrespect... I mean this as a matter of serious inquiry. Where's the harm? What's the trade off?

Though I appear the lone dissenter in thinking isset and empty checks in this context seem more a matter of style than best practice,
I remain hopeful that someone may help me discover the critical error(s) in my logic.😕

Anyone?