views:

1024

answers:

5

Hi

I'm fairly new to PHP and am creating a website for my company. I am using some existing code that I have obtained but can't seem to make it work properly - any help would be much appreciated!

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 to 3 and display the relevant products. However, despite setting $id, it still defaults to 0. Is there something obviously incorrect with my code?

+4  A: 

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
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
The call setdefault($_GET['id'], 0) can still generate a warning if id is not an index in $_GET.
jmucchiello
you could improve this and not generate a warning by using array_key_exists('id', $_GET)
jonstjohn
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
A: 

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).

R. Bemrose
A: 

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;
zodeus
A: 

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
+1  A: 

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