Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This code gets me where I need to be, but my word it is ugly.

I am checking for the existence of two $_GET variables with if() statements. I am setting the variables to empty strings first so I do not get an error when I echo them back.

$getvar1 ='';
$getvar2 ='';
$np ='';

if(isset($_GET['getvar1'])) $page_id = $_GET['getvar2'];
if(isset($_GET['getvar2'])) $route = $_GET['getvar2']; 

if(($getvar1 == '' && $getvar1 == '') || $getvar1 == '4')  $np = 'np'; 

    echo "$getvar1 $getvar2 $np"; 

Is there a better way to declare the variables than setting them to empty strings?

Is there a better way to check and set the $_GET variables?

share|improve this question

3 Answers 3

up vote 2 down vote accepted

If it's a small app something like that would probably be better:

$getvar = isset($_GET['getvar']) ? $_GET['getvar'] : 'somedefault';

If you want to test more stuff:

$getvar = isset($_GET['getvar']) && $JUST_PUT_IT_HERE ? $_GET['getvar'] : 'somedefault';
share|improve this answer

Array Merge

If you're just setting defaults, you could use array_merge():

$defaults = array(
    'getvar1' => 'default setting', //a default value
    'getvar2' => false //now you can just test
);

$params = array_merge($defaults, $_GET); //any missing elements get the default value

echo $params['getvar1']; //will output 'default setting' if $_GET['getvar1'] was not set

if($params['getvar2']){
     //do something if $_GET['getvar2'] was set to a non-false value
}

It should be noted that this only works with things like $_GET - that are already arrays. The ternary method, a getOrDefault() function, or some enigmatic logic operator code is better suited when setting defaults not already in an array.

Logic Operators

Here's an example of using logic operators (similar to using ternary operator) from the default Zend MVC index.php:

defined('APPLICATION_PATH')
    || define('APPLICATION_PATH', realpath(dirname(__FILE__) . '/../application'));

Using the question's example, it would look something like this:

isset($_GET['getvar1']) || $_GET['getvar1'] = 'default value';
share|improve this answer
    
Nice trick, I'll note that one! –  mAu Aug 4 '11 at 20:47
    
@mAu Thanks, updated with some related information. –  Tim Lytle Aug 5 '11 at 15:07

How about writing a function like this:

function getOrDefault($index, $default)
{
    return (!isset($_GET[$index]) || empty($_GET[$index]) ? $default : $_GET[$index]);
}

$getVar1 = getOrDefault('getvar1', '');
$getVar2 = getOrDefault('getvar2', '');

This function should be as general as possible and be located somewhere in a function-library. I don't know about the rest of your project. But for example if you do have some kind of request object (containing all information about the current (http)request), you could write specialized versions:

class Request()
{
    public function getPost($param, $default = '')
    {
        if(!isset($_POST[$param]) || empty($_POST[$param])
        {
            return $default;
        }
        return $_POST[$param];
    }

    // If param is set both in $_GET and $_POST, $_GET is return.
    public function getParam($param, $default = '')
    {
        if(!isset($_GET[$param]) || empty($_GET[$param))
        {
            return $this->getPost($param, $default);
        }
        return $_GET[$param];
    }
}

$getVar1 = $request->getParam('getVar1', '');
$getVar2 = $request->getParam('getVar2', '');
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.