1

I have a variable, $id, which identifies the product category type to display on the page. I first need to check that the id variable has been set and if not, default the variable to category 0.

The code I have is as follows:

setdefault($id, 0);

function setdefault(&$var, $default="") 
{
   if (!isset($var)) 
   {
      $var = $default;
   }
}

So with the address www.website.com/browse.php, I would expect it to default to $id = 0.

With the address www.website.com/browse.php?id=3, I would expect it to set $id = 3 and display the relevant products.

However, despite setting $id, it still defaults to 0. Is there something obviously incorrect with my code?

mickmackusa
  • 43,625
  • 12
  • 83
  • 136
Tray
  • 3,105
  • 8
  • 26
  • 25

6 Answers6

4

You are probably expecting PHP to use the $_POST and $_GET as global variables. PHP used to be setup this way, back in the day, but newer versions require you to explicitly reference these variables.

You could try this:

setdefault($_GET['id'], 0);

function setdefault(&$var, $default="") 
{
   if (!isset($var)) 
   {
      $var = $default;
   }
}

or even more simply (using the ternary operator):

$id = array_key_exists('id', $_GET) ? $_GET['id'] : 0;
jonstjohn
  • 59,650
  • 8
  • 43
  • 55
  • Thank you. It appears the code I have been given to look through is somewhat out of date now!! After taking your advice, the code works perfectly :) – Tray Mar 02 '09 at 18:49
  • The call setdefault($_GET['id'], 0) can still generate a warning if id is not an index in $_GET. – jmucchiello Mar 02 '09 at 19:44
  • you could improve this and not generate a warning by using array_key_exists('id', $_GET) – jonstjohn Mar 02 '09 at 19:53
  • isset will not generate a warning. It is a language construct not a normal function: http://us2.php.net/manual/en/function.isset.php – jmucchiello Mar 02 '09 at 21:06
1

First off, if this is PHP 5.X I highly recommend you do not pass variables by reference using the &. That being said. the isset function call will always be true withing the function. But you will receive an undefined variable warning on the setdefault($id, 0);

Try this instead.

$id = isset($id) ? $id : 0;
Hawk Kroeger
  • 2,336
  • 17
  • 21
1

If $id is not set, the call to setdefault($id,0) will generate a warning. A function like setdefault does not work in PHP. Use this instead.

if (!isset($id)) $id = 0;

If you are doing this for array variables, like $_GET and $_POST, you can do this:

function getuservar($A, $index, $default = '') {
    if (!isset($A[$index])) return $default;
    if (get_magic_quote_gpc()) return stripslashes($A[$index]);
    return $A[$index];
}

$clean_id = getuservar($_GET, 'id', 0);

This return form is better because you stop using the $_GET array immediately and only use the variables that are cleaned in the rest of the code. You clean your external variables once and never touch the external variables again in your code. $A can be $_GET, $_POST, $_REQUEST or $_COOKIE.

It also handles the annoying magic_quote stuff for you so you know the data in the variable is the text sent by the user. Just remember to clean it again when sending it back to the user or to the database.

jmucchiello
  • 18,754
  • 7
  • 41
  • 61
0

How is $id being set? If register_globals is off (and it's off by default in PHP 4.2 and newer), you need to look at $_GET['id'] or $_POST['id'] instead (or $_REQUEST['id'], but there are reasons to avoid that).

Powerlord
  • 87,612
  • 17
  • 125
  • 175
0

The code to set $id to sometime from the query string (browse.php?id=3) would also need to be included. You might be thinking of the register_globals settings that PHP has that will automatically create variables when included in the query string. DO NOT re-enable that feature, for several years now that has been shown to be a really bad idea.

Any variable you are pulling from the query string needs to be checked for type/safety before using it. So for example you might pull the variable from the $_GET super global, check to see if it's numeric, if it is not then set the default:

if (!is_numeric($_GET['id']) {
  setdefault($_GET['id'], 0);
}
acrosman
  • 12,814
  • 10
  • 39
  • 55
0

You do not need to write a helper function for this.

If you want to declare $id from $_GET['id'] and fallback to zero, use the following:

$id = $_GET['id'] ?? 0;

Of course, this does not validate that $id contains an integer value from the user. You'll need to validate the value as the next step.

mickmackusa
  • 43,625
  • 12
  • 83
  • 136