PHP Custom $_GET checker function performance

Question

I'm trying to follow the DRY programming philosophy and I am tired of running isset() and an if not empty statements for each time I want to check a $_GET[] variable.

I am working on a page that receives up to 4-5 GET parameters in the URL, so I wrote the following function to reduce repetitive code:

function getset( $get, $default, $empty=FALSE ) {
   // checks if a GET parameter is set
   if ( isset($_GET[$get]) ) {
      // optionally allows empty values
      if ($empty) { return $_GET[$get]; }
      // check if value is not empty
      else { 
         if ( $_GET[$get] != '' ) { return $_GET[$get]; }
         // otherwise return default value
         else { return $default; }
      }
   }
   // if param not set then return default value
   else { return $default; }
}

// set the variables
$foo = getset('foo', 'abc');
$bar = getset('bar', 'xyz', TRUE)

// localhost/?foo    => returns 'abc'
// localhost/?foo=x  => returns 'x'
// localhost/?bar    => returns TRUE / ''

I am wondering if my function is proper, efficient and safe compared to the alternative:

// default values
$foo = 'abc';
$bar = '';

// set $foo from $_GET value
if ( isset($_GET['foo']) && $_GET['foo'] != '' ) {
    $foo = $_GET['foo'];
}
// set $bar from $_GET value
if ( isset($_GET['bar']) ) {
    $bar = $_GET['bar'];
}

I am open to suggestions for improving/refining my code.

Answer 1

If your aim is to go DRY, I would consider taking it one step further. You are writing a function for a common use case indeed, but one could wonder if this is only used for $_GET variables. What about $_POST or $_SESSION or some none magic array your application uses...

I'm used to working with Laravel, and this framework offers a very basic support method called array_get (docs here). I use it so often, it should really be part of core php.

Allow me to rewrite your function to this API. (Note that I'll leave . syntax for nested arrays out, though it has proven useful to me it would take us to far.)

function array_get($array, $key, $default = null) {
  if (! array_key_exists($key, $array)) {
    return $default;
  }

  if (is_null($array[$key])) {
    return $default;
  }

  return $array[$key];
}

// usage
$get = array_get($_GET, 'myKey', 'default');
$post = array_get($_POST, 'myKey', 'default');
$session = array_get($_SESION, 'myKey', 'default');
$value = array_get($myArray, 'myKey', 'default');

I'm not sure about you $empty parameter. It doesn't feel entirely right to me to have it in there, but I may be wrong.

If you decide to stick with your function, try to clean it up a bit.

• I feel the comments are obsolete when you are performing such trivial, perfectly readable tasks.
• All the else statements are obsolete, they just clutter the code.
• Nesting control structures is rarely a good idea. It makes the code harder to read. You'll be surprised how rarely you have to nest control structures if you take this rule into account. If you have to, you are probably doing something else wrong (single responsibility? extract another function perhaps)
• Nice job on the early returns!

Have a look at this:

function getset( $get, $default, $allowEmpty=FALSE ) {
    if (! isset($_GET[$get]) ) {
        return $default;
    }

    if ($allowEmpty) { 
        return $_GET[$get]; 
    }

    if ( $_GET[$get] === '' ) {
        return $default;
    }

    return $_GET[$get];
}

update:
Concerning your question about the $allowEmpty, I think you are approaching the problem from the wrong angle. Either you want a variable to have a value, or you want to default a missing variable.

// default
$foo = array_get($_GET, 'foo', 'default');

// required
$bar = array_get($_GET, 'bar');
if (is_null($bar)) {
  // required var missing
}

I feel cramming that $allowEmpty into the function violates he SRP somewhat. You are not just getting a value from an array anymore, you are validating it as well.

Do feel free to add it however, it is your code after all. It should be as simple as adding a fourth parameter to the array_get function and checking it similar to the way I provided in the refactored getset function.