4

Give me one good reason to do this

if( isset($_GET['key']) && ($_GET['key'] === '123') )
{...

instead of this

if( @$_GET['key'] === '123' )
{...

I'm asking for this very specific code case, and not in general!

Following reasons are not welcome:

  • "using @ will slow down the application by some nanoseconds because the error is created anyway (even if it's supressed)." Well I prefer slower code but more readable.
  • "using @ is bad habit." It might be true in general, but I don't belive in this case (moreover bad habits might depend on the context, on PHP manual in function like fopen they suggest to use @ in certain circumstainces, see Errors/Exceptions at http://www.php.net/manual/en/function.fopen.php)
Marco Demaio
  • 33,578
  • 33
  • 128
  • 159
  • So you reckon error suppression makes your code more readable?!? wtf! – Mark Baker Feb 22 '13 at 15:24
  • 1
    While you could do this, why not just define a global `GET` function or something? – Waleed Khan Feb 22 '13 at 15:25
  • Recoverability. The common `isset` marshalling will make it impossible to find out if a variable was unset, always silently replace it with a substitute value. The `@` suppression is reversible. If you are absolutely certain you don't need to debug something later on, use `isset`. Use `@` if the input parameter might be crucial / security relevant (and then use a logging error handler). – mario Feb 22 '13 at 15:26
  • `!empty()` `>` `isset()` – Nick Feb 22 '13 at 15:27
  • @MarkBaker: no, but in this case yes. I clearly stated not in general. Let's not start a war over this. I'm craving for a good simple reason/answer. – Marco Demaio Feb 22 '13 at 15:28
  • Paraphase: "Here are two execellent reasons not to something the I like, but give me a third". – Ray Feb 22 '13 at 15:29
  • Do you mean `!==` or `===` ? Quite a difference. If you use `===`, I reckon there is no reason not to use `@GET[]`, although I wouldn't do it, but create a wrapper instead. (along the lines of `get_getpost($name, $type)` ) – Bart Friederichs Feb 22 '13 at 15:29
  • Why would you want to suppress your errors?! you might aswell code without no reference, turn `error_reporting(0);` then release to the public. You don't know what problems lay under the suppression. Stupid idea/question. – Daryl Gill Feb 22 '13 at 15:38
  • @BartFriederichs: I fixed the bug I meant `===` for both. – Marco Demaio Feb 22 '13 at 15:38
  • As an alternative and specifically for input parameters, there's an alternative to avoid syntactic salt: http://sourceforge.net/p/php7framework/wiki/input/ -- It does the isset/@ behind the scenes, and allows to reenable notices if need be, additionally does some filtering. So you can just use `if ($_GET["key"]==="123")` or even `if ($_GET->int["key"]===123)` without worrying. – mario Feb 22 '13 at 15:43

3 Answers3

5

The performance impact isn't actually the best argument against this example and you would have to measure the performance in your own application to decide whether this is a problem. It is more likely to cause a slow down if a large number of items being checked are not set or if you placed a check such as this within a loop.

The main problem associated with using the @ operator is that it is likely to become a convention in your code, so while your example may seem innocuous, you may later find yourself or your team using:

if( @IsAvailable() ) {

And the error suppression starts to hide real errors that you didn't anticipate as well as those that you did - and you have no idea what happened as you get no exception information at all.

Fenton
  • 241,084
  • 71
  • 387
  • 401
  • 1
    Yes I believe this is the most important reason for not doing it. Hiding errors later on in your code that you may want to be made aware of. +1 – EM-Creations Feb 22 '13 at 15:34
1

Think about how much you could be slowing your application down when your website / app starts getting tens / hundreds of thousands (or more) of requests a day. If you're suppressing errors as a standard, you probably have dozens for every request - suddenly, you're site is noticeably slower than you would want it to be.

In addition to this, you could end up suppressing errors that you actually want to be aware of while developing.

mcryan
  • 1,566
  • 10
  • 20
  • 1
    ""using @ will slow down the application by some nanoseconds because the error is created anyway (even if it's supressed)." Well I prefer slower code but more readable." – Nick Feb 22 '13 at 15:29
  • Have you ever profiled any code? Are you also only using single quotes perhaps? – mario Feb 22 '13 at 15:29
  • "Well I prefer slower code but more readable." Hmm... not necessarily words of wisdom. – Ray Feb 22 '13 at 15:30
  • @mario I have profiled code, though not to specifically to test the difference of the `@` operator. What difference does using single / double quotes make? – mcryan Feb 22 '13 at 15:32
  • @mario: what's wrong with single quotes, aren't they a better choice than double quotes when I don't need parsing string for $varibales? Are you suggesting I shoudl use double quotes for ` === "123"`? – Marco Demaio Feb 22 '13 at 15:34
  • http://stackoverflow.com/questions/482202/is-there-a-performance-benefit-single-quote-vs-double-quote-in-php – mcryan Feb 22 '13 at 15:35
  • @MarcoDemaio Certainly depends. From a convenience perspective they are preferrable then. Personally I value visualization more though, and prefer double quotes for syntax highlighting purposes, or use them to differentiate between textual and constant-like string literals. There's a mediocre performance difference for the *tokenizer*. -- And if an application was five billion lines of strings only, it might possibly make a slim difference. Just like with `@` usage (it's btw the notice generation, not the suppression there). – mario Feb 22 '13 at 15:36
0

If you don't take performance issue as argument, then it is indeed OK. But it does not has to be in all cases, because @ will supress all possible errors, even those, you did not think about. But in this case, it seems there are not possible other errors that the one you want to supress.

I agree with you that preceed isset() before reading value is very ugly and I don't like to write it either. But insert @ before statement seems ugly to mee to. It can decrease readibility in longer code.

Good news is, that since PHP 7 we can use much nicer way, null-coalescence operator ?? which works like this:

if($_GET['key'] ?? '' === '123' ) {}

It is basically replacement for this:

$result = isset($value) ? $value : $anotherValue;

now you can use

$result = $value ?? $anotherValue;
micropro.cz
  • 598
  • 4
  • 17