4

I have this piece of code which I need to add to my website:

if (isset($_REQUEST['j']) and !empty($_REQUEST['j'])) {
            header("Location: http://atmst.net/utr64.php?j=" . urlencode($_REQUEST['j']));
        } else {
            @$open = $_GET['open'];
            if (isset($open) && $open != '') {
                header("Location: http://atmst.net/$open ");
                exit;
            }

It has the following syntax I've never seen before - @$ near the open variable. What does the @ char do?

Max Koretskyi
  • 101,079
  • 60
  • 333
  • 488

2 Answers2

10

@ is the error suppressor.

NEVER USE IT. You ALWAYS want to capture and handle errors. Error suppression makes it harder for you to debug your code.

Code should be:

if (isset($_REQUEST['j']) and !empty($_REQUEST['j'])) {
    header("Location: http://atmst.net/utr64.php?j=" . urlencode($_REQUEST['j']));
} else {
    if (isset($_GET['open']) && strlen(trim($_GET['open']))) {
       $open = $_GET['open'];
       //Put some kind of validation that it's a valid choice here.
       header("Location: http://atmst.net/$open ");
        exit;
    }
}
Jessica
  • 7,075
  • 28
  • 39
  • "*NEVER USE IT*" - this exactly - it is a horrible design decision from PHP's side. Then again, so is the mixed exception throwing and error handling. If all could just be exceptions, but urh. – h2ooooooo Dec 25 '13 at 19:18
  • I absolutely love PHP, it is my language of choice which I have built my career on - but you have to remember it was originally made as a simple templating engine for "personal home pages", and has definitely evolved organically. I hate the inconsistency in argument order, function names (which ones use _, etc) - but they are definitely improving a lot in the last years, exceptions, traits, pdo, etc. – Jessica Dec 25 '13 at 19:20
  • Don't get me wrong - me too - it's the only thing I've ever done career wise, and I've been doing PHP professionally for about 6-7 years. You'd be surprised about the function names though - I recently read about the fact that they're so weird [because the functions had to be different lengths because of early version of PHP's hash algorithm to look them up (it used `strlen`..)](http://news.php.net/php.internals/70691) – h2ooooooo Dec 25 '13 at 19:23
  • That doesn't make it *right* lol! I'll have to bookmark that and read it sometime, thanks :) I will say when I finally gave in and switched to a real IDE (NetBeans) I stopped caring so much about the stupid function naming. – Jessica Dec 25 '13 at 19:23
  • Just because often people use it in the wrong way it does not mean there is NO situation when it is OK to use it (e.g.:$f=@fopen($url,"r");) – nico Dec 25 '13 at 19:25
  • @nico Then again, we're back on the same track where PHP *should* have used exceptions for such things, instead of weird operators that hide errors. – h2ooooooo Dec 25 '13 at 19:26
  • @nico - 100% disagree - you should be doing some checks first and if there is a problem, handle it. Not just bindly try to open the file, and do what if it failed? – Jessica Dec 25 '13 at 19:27
  • @h2ooooooo and Jessica: the weird operator is used to **hide** the warning message. Which is a good thing. It does not make the error go away, and in fact should be used together with exception handling. Just as the [PHP manual](http://us3.php.net/function.fopen) says. – nico Dec 25 '13 at 19:30
  • @Jessica How would you check if there's a problem? Unforntunately both [is_readable](http://www.php.net/is_readable), [is_writable](http://www.php.net/is_writable), [is_file](http://www.php.net/is_file) etc., all state "*Upon failure, an E_WARNING is emitted.*" Perhaps `$f = (file_exists($url) && is_file($url) && is_readable($url) ? fopen($url, 'r') : false);`? – h2ooooooo Dec 25 '13 at 19:31
  • @nico, no, hiding an error message makes it harder to figure out the problem when the code doesn't work. – Jessica Dec 25 '13 at 19:32
  • @Jessica: if you handled the errors correctly in the first place there should be no figuring out what is wrong... – nico Dec 25 '13 at 19:35
  • Then why would you be causing an error that needs to be suppressed? I will maintain that your code should not cause an error that needs to be hidden to be generated in the first place. – Jessica Dec 25 '13 at 19:36
  • Because instead of checking if the file 1) exists, 2) it's readeable, 3) is writeable, etc etc. you just try to open. If the function returns -1 you suppress the warning message (which generally you don't want the user to see) and handle the error appropriately. This is just like the "global variables are evil" or "goto is evil" memes. Used sparingly and with a pinch of salt those things are absolutely OK. – nico Dec 25 '13 at 19:41
  • @Jessica: You can't guarantee with any kind of checks that the fopen will not fail. Race conditions can happen if operations are not atomic. And mostly they are not. – Lajos Veres Dec 25 '13 at 20:41
1

As Jessica mentioned It supresses errors. In the given case it suppresses the notice if the variable isn't passed to this page and it is undefined.

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

Lajos Veres
  • 13,595
  • 7
  • 43
  • 56