views:

39

answers:

2

I learned how to access by reference in another answer on stackoverflow, but cannot find it again. Anyways, is the following method unsafe or at all unreliable?

protected function checkVar($requestType, $varname, $checkIfNumber = false)
{
    switch($requestType)
    {
        case 'GET':
            $sg = &$_GET;
            break;
        case 'POST':
            $sg = &$_POST;
            break;
        default:
            throw new Exception('Variable `$requestType` is not `GET` or `POST` in AController::checkVar().');
    }

    if(!isset($sg[$varname])) {
        throw new Exception("$requestType variable [$varname] is not set in AController::checkVar().");
    } else if(empty($sg[$varname])) {
        throw new Exception("$requestType variable [$varname] is empty in AController::checkVar().");
    } else  if($checkIfNumber) {
        if(!ctype_digit($sg[$varname])) {
            throw new Exception("$requestType variable [$varname] is not a number in AController::checkVar().");
        }
    }   

    return $sg[$varname];
}
+2  A: 

That's why we have the $_REQUEST superglobal.

protected function checkVar($varname, $checkIfNumber = false)
{
    if(!isset($_REQUEST[$varname])) {
        throw new Exception("variable [$varname] is not set in AController::checkVar().");
    } else if(empty($_REQUEST[$varname])) {
        throw new Exception("variable [$varname] is empty in AController::checkVar().");
    } else  if($checkIfNumber) {
        if(!ctype_digit($_REQUEST[$varname])) {
            throw new Exception("variable [$varname] is not a number in AController::checkVar().");
        }
    }   

    return $_REQUEST[$varname];
}

I know it's not strictly the same thing, but IMHO it's good enough.

NullUserException
@NullUserException - I didn't know about $_REQUEST! So, if I understand correctly, as long as I check which request method has been used before calling your example, then this will work for GET, POST, and COOKIE??
letseatfood
@NullUserException - Thank-you for your excellent answer. I chose @Wrikken's answer since it was about the main point, which was using reference.
letseatfood
@lets No sweat :)
NullUserException
+3  A: 

This is not how you should use references. A 'copy' operation is actually cheaper as long as the values don't change, and there is no reference needed here (especially as you're not returning by reference, but actually making a copy). The only thing references in this point of the code can do is cause obscure errors later on which can be quite hard to track down.

Wrikken
Interesting that it is cheaper!
letseatfood
It's a reference counting thing, which is a whole action not needed when there is none.
Wrikken