Is this sort of thing considered OK in PHP?
$foo = $_GET['foo'];
$foo = empty($foo) || !custom_is_valid($foo) ? 'default' : $foo;
Are there cleaner alternatives to this? I'm basically trying to avoid extra table look-ups.
Add a comment
|
5 Answers
Does custom_is_valid() check for an empty variable? Because being able to remove the empty() and "or not" would go a long way to improving that code.
2 Comments
chaos
It will still throw a warning if
$_GET['foo'] isn't there.
andres descalzo
only use : "$foo = custom_is_valid($_GET['foot']);", and in custom_is_valid(..) check data, ¿ok?
As you'll see if you turn error_reporting(E_ALL) on, that isn't really the best way to do it. PHP basically wants you to do
$foo = empty($_GET['foo']) || !custom_is_valid($_GET['foo']) ? 'default' : $_GET['foo'];
5 Comments
chaos
I can't imagine what makes you think there's a PHP specification. :/
chaos
Ahh. :) Yeah, PHP is, um, not very rigorously defined. I doubt performance on those three lookups will be awesome, but it'll be better than if you make PHP generate a warning (whether or not display of it is suppressed). You should basically always run with
error_reporting(E_ALL | E_STRICT) if you care about performance, and eliminate anything that PHP complains about, because generating those warnings is quite slow.
bobince
In reality the extra lookup costs are minuscule compared to everything else a typical script does; you won't be able to measure any difference.
How about:
$foo = 'default';
if (array_key_exists('foo', $_GET) and custom_is_valid($_GET['foo'])) {
$foo = $_GET['foo'];
}
And don't be afraid of the array lookups, they are not that slow :)
Comments
Perhaps instead of just checking if it is valid, run it though a cleaning function that takes a default.
Also, I like to use the following function so I don't get warnings on accessing non-existant array keys when running E_STRICT:
function GetVar($var, $default = '') {
$value = $default;
if(isset($_GET[$var])) {
$value = $_GET[$var];
}
return $value;
}
function custom_clean($value, $default = '') {
... validation logic or return $default ...
}
$foo = custom_clean(GetVar('foo'), 'default');
Comments
A class here would make your life a lot easier.
<?php
class ParamHelper
{
protected $source;
public function __construct( array $source )
{
$this->source = $source;
}
public function get( $key, $default=null, $validationCallback=null )
{
if ( isset( $this->source[$key] ) && !empty( $this->source[$key] ) )
{
if ( is_null( $validationCallback ) || ( !is_null( $validationCallback ) && call_user_func( $validationCallback, $this->source[$key] ) ) )
{
return $this->source[$key];
}
}
return $default;
}
}
// Just for the demo
function validateUpper( $value )
{
return ( $value == strtoupper( $value ) );
}
// Mimic some query-string values
$_GET['foo'] = 'bar';
$_GET['bar'] = 'BAZ';
$_GET['lol'] = 'el oh el';
$getHelper = new ParamHelper( $_GET );
echo $getHelper->get( 'foo', 'foo default', 'validateUpper' ), '<br>';
echo $getHelper->get( 'bar', 'bar default', 'validateUpper' ), '<br>';
echo $getHelper->get( 'baz', 'baz default' ), '<br>';
echo $getHelper->get( 'lol' ), '<br>';
echo $getHelper->get( 'rofl' ), '<br>';
