CODEWITHSUNDEEP
phperror-handling
Zelda
Zelda

Reputation: 2124

When to suppress errors in PHP

I've got some variables that may or may not be defined (an $isLoggedIn boolean) and I'm trying to clean out the error messages. I was wondering if there was any reason I shouldn't use the error suppressor operator:

if (@$isLoggedIn)

Or if I should instead check the existance of the variable first:

if (isset($isLoggedIn)and$isLoggedIn)

In a production environment is there any drawback/benefit to either approach in particular? The functionality of both statements is identical and there are no problems caused by this var being undefined. It shouldn't be logged as an error though.

Upvotes: 4

Views: 304

Answers (8)

symcbean
symcbean

Reputation: 48387

You should never, ever allow PHP to write error messages to the browser on a public website. You should always log them though, and wherever possible you should handle them in your code - even if it is by way of set_error_handler().

Now we've got that out of the way....

This is a warning, not an error.

Yes the warning messages can be a PITA sometimes. But some people like strict variable checking. Using the suppression operator provides a more targeted approach than suppressing all E_STRICT messages, so even though I love PHP's support for allowing the use of variables withot declarations, this is what I do (and use try{} catch() where it's appropriate too).

So use either method where it is apropriate - but do LOG these warnings when they are not suppressed - these are things you need to fix - even if the fix is to just add suppression.

Upvotes: 1

GolezTrol
GolezTrol

Reputation: 116160

The best option, I think, is not to use the basic warning system of PHP, but use the available exception handling where possible. Translate warnings to exceptions too, so you can handle everything in the same way.

You can catch specific errors. Global errors that are not caught by a specific handler can and should be caught by a general handler. That way, you can catch all these errors and report them in a log and/or mail them to the development department (or to yourself).

To the user, you should not show these errors in a production environment, especially when they contain SQL-errors or messages about missing variables. Make the page die gracefully, by showing some general content and a general, user friendly error message (if you have to), but keep the technical stuff hidden.

But never ignore or hide exceptions at all, because it will make debugging a living hell.

Upvotes: 0

Mārtiņš Briedis
Mārtiņš Briedis

Reputation: 17762

If I'm not certain if a variable is set, but I want to know if it is true then I write:

if(!empty($var)){}

But still, you should structure your code so that you are always certain about your variables. You should avoid global variables.

Upvotes: 0

Jose Vega
Jose Vega

Reputation: 10258

http://php.net/manual/en/language.operators.errorcontrol.php

Your variable should always exist, at least initialize it to false and once logged in, set it to true. In terms of suppressing, I would recommend going with the second method.

Warning

Currently the "@" error-control operator prefix will even disable error reporting for critical errors that will terminate script execution. Among other things, this means that if you use "@" to suppress errors from a certain function and either it isn't available or has been mistyped, the script will die right there with no indication as to why.

Upvotes: 1

matthewdaniel
matthewdaniel

Reputation: 1476

Have a bunch of error suppressors in your code makes maintenance harder especially for people other than yourself. The next person now has to learn to look at the code in a different way that he is used to. The suppress is just a hack and will cause other people to try to figure out why there is a hack which could slow down any debugging efforts.

Doing an if(@$var) produces an identical result to if(!empty($var)) so just use that instead since it is the more semantically correct way of checking it.

Upvotes: 1

Sascha Galley
Sascha Galley

Reputation: 16091

The most logic way would be to always declare those variables - especially an important one like $isLoggedIn - and check if it is true:

if ($isLoggedIn)

or only set it, if it is true:

 if (isset($isLoggedIn)

In my opinion the first option is better and mixing both is not a good solution!

Upvotes: 0

Endophage
Endophage

Reputation: 21483

In my experience, you should never suppress errors on an individual level. In your production environment set error reporting to 0 and display errors to off. Having individual supressions like this is shooting yourself in the foot when you come back to fix an error in 6 months, forget it's there and can't work out why no errors are showing up but something is clearly broken.

In you particular case, just set $isLoggedIn to false then override that value at the location you are currently instantiating it (I'm assuming code structure here).

Upvotes: 3

Incognito
Incognito

Reputation: 20765

If you're using a variable, it should exist. Your code shouldn't suppress errors, you should fix them or deal with them.

Upvotes: 3

Related Questions