Page 1 of 1
How should $_GET be checked
Posted: Mon Apr 16, 2007 6:22 am
by ngungo
Let's say a potential url is something like:
http://mysite.com/myphp.php?param=foo
How and when (what diiference) should the $_GET['param'] be checked?
Code: Select all
if ($_GET['param']) == '') die('error');
else dosomething();
or
Code: Select all
if (isset($_GET['param'])) dosomething();
else die('error');
or
Code: Select all
if ($_GET['param'] == 'foo') dosomething();
else die('error');
or
Code: Select all
if (isset($_GET['param']) && ($_GET['param'] == 'foo')) dosomething();
else die('error');
or any other method?
Posted: Mon Apr 16, 2007 6:42 am
by Xoligy
The last one. All the others, except no2. which isn't specific enough, will create a notice and you should code with notices on, it makes life a lot easier.
Posted: Mon Apr 16, 2007 6:44 am
by Oren
Certainly the last one.
Posted: Mon Apr 16, 2007 7:32 am
by ngungo
What difference is between 3 & 4 (last 2)? Is that isset() of the last one redundant or overkill?
Posted: Mon Apr 16, 2007 7:46 am
by Oren
ngungo wrote:Is that isset() of the last one redundant or overkill?
No, it's not.
If you use #3 instead of #4 and the user doesn't send $_GET['param'] in the URL then PHP will generate a notice similar to this:
Notice: Undefined index: param in {path_to_script/script}.php on line {line_number_here}
P.S the {...}'s are just placeholders

Posted: Mon Apr 16, 2007 9:52 am
by ngungo
So would it be fair to say ... in general, always check isset first then check the value?
Posted: Mon Apr 16, 2007 10:02 am
by Maugrim_The_Reaper
So would it be fair to say ... in general, always check isset first then check the value?
isset() checks if a variable exists. It the variable does not exist, then there not much else to do (which is why it's always the first part if the "if" condition). So yes, it would be fair to say an isset() should be done first.
Posted: Tue Apr 24, 2007 10:49 am
by mpeacock
This:
Code: Select all
if (isset($_GET['param']) && ($_GET['param'] == 'foo')) dosomething();
else die('error');
Is similar to some pretty standard Java:
Code: Select all
if (request.getParameter("param") != null && request.getParameter("param").equalsIgnoreCase("Foo"))
{
...
}
Which is better abbreviated to:
Code: Select all
if ("Foo".equalsIgnoreCase(request.getParameter("param"))
{
...
}
So, in PHP, I bet you could dispense with the issset checks when you have a string literal you can check against, since it's doesn't really matter if the param is set. You
really want to know if it's equal to the literal "foo".
This is probably what I'd do - since I rarely want my scripts to care about the request type:
Code: Select all
if ('foo' == $_REQUEST['param']) dosomething();
else die('error');
Posted: Tue Apr 24, 2007 3:04 pm
by feyd
mpeacock wrote:So, in PHP, I bet you could dispense with the issset checks when you have a string literal you can check against, since it's doesn't really matter if the param is set. You
really want to know if it's equal to the literal "foo".
This is probably what I'd do - since I rarely want my scripts to care about the request type:
Code: Select all
if ('foo' == $_REQUEST['param']) dosomething();
else die('error');
...which will promptly fire a E_NOTICE level error if $_REQUEST['param'] does not exist. Stick to
isset().
Posted: Tue Apr 24, 2007 5:47 pm
by Ollie Saunders
Code: Select all
if (isset($_GET['param']) && ($_GET['param'] == 'foo')) dosomething();
I'm pretty sure that's going to generate an E_NOTICE because the extra parenthesis forces the equality comparison to run before the isset.
2000th post!
Posted: Wed Apr 25, 2007 4:57 am
by dillion
would it not be better to do:
Code: Select all
$param = (isset($_GET['param'])) ? ($_GET['param']) : "";
if ($param == 'foo') dosomething();
else die('error');
?
Posted: Wed Apr 25, 2007 6:10 am
by Oren
ole wrote:Code: Select all
if (isset($_GET['param']) && ($_GET['param'] == 'foo')) dosomething();
I'm pretty sure that's going to generate an E_NOTICE because the extra parenthesis forces the equality comparison to run before the isset.
2000th post!
No it won't, '&&' is a short-circuit AND - meaning it stops on the first FALSE.
Posted: Wed Apr 25, 2007 6:22 am
by aaronhall
No notice:
Code: Select all
if((isset($asdf)) && ($asdf == 'asdf')) echo 'not true';
Notice:
Code: Select all
if(($asdf == 'asdf') && (isset($asdf))) echo 'not true';