Using extract

Question

I'm new to OOP and want to revamp this function to get rid of using globals.

    function CatchListing() {

    $parseform = array('itemnum','msrp','edprice','itemtype','box','box2','box25','box3','box4','box5','box6','box7','itemcolor','link'); 

    foreach ($parseform as $globalName) {
     $GLOBALS[$globalName] = mysql_real_escape_string($_POST[$globalName]);
    }


}

I was told to use array_map & then extact, but I am not sure of how to structure this.

    function CatchListing() {
 $_POST['listing'] = array_map('mysql_real_escape_string', $_POST);
 $nst = extract($_POST['listing']);
}

(listing is the form name btw)

Thanks

Answer 1

Be VERY careful about using extract with externally inputted values as from $_GET and $_POST.

you're much better off extracting the values manually to known values.

It's far too easy for an extract from _GET or _POST to clobber existing variables.

Answer 2

There are so many things to say and Jonathan makes a very good start. Every time the user has the opportunity to play with your internal data and you don't check them, there is a huge "opportunity" (depends on the view..) that something goes wrong. Here is another approach on how to "maybe" get where you want to go:

<?php

function Sanitize($string){
  return mysql_real_escape_string(trim($string));
}

function CatchListing(){  
  foreach($_POST as $key => $value) {
    $key = Sanitize($key);
    $value = Sanitize($value);
    if($key && $value && !$GLOBALS[$key]){ /* prevent overwriting existing globals*/
      $GLOBALS[$key] = $value;
    }
  }
}

global $nice;
$nice = "working";

CatchListing();    

print_r($GLOBALS);

?>

To be honest, it still has not really anything to do with OOP and furthermore should be seen as a procedural approach. Personally I would use an additional and reusable function to "sanitize" the input, because you never know, if someday you want to change your database or the "escape" function and then you exactly know where to look for possible changes. Ah one more thing: Are you certain that you don't know all the possible names of all the variables you have to expect? Maybe you can predetermine them and put them in another array and check each user supplied argument with in_array.

Answer 3

To completely get rid of the usage of globals in your code, and also to make it much better overall, you can do something along these lines:

• stop using $_POST, as it's a superglobal. When code needs values from superglobals, pass them as parameters
• don't store values into $GLOBALS. If you need to return more than one value, consider returning an object or an array

Here's how I think I would modify your code to improve it:

function CatchListings($listings) {    
    $filteredListings = array_map('mysql_real_escape_string', $listings);

    //I assume you only need the values in the array in the original snippet,
    //so we need to grab them from the parameter array and return only that
    $requiredListings = array();
    $requiredKeys = array('itemnum','msrp','edprice','itemtype','box','box2','box25','box3','box4','box5','box6','box7','itemcolor','link');
    foreach($requiredKeys as $key) {
        $requiredListings[$key] = $filteredListings[$key];
    }

    return $requiredListings;
}

To use this function, you simply do $result = CatchListings($_POST);. Same result, no globals used.

There is one thing to consider, though. It may not be best possible form to just pass a randomly filled array (ie. $_POST) to the function, and expect it to contain specific keys (ie. $requiredKeys array). You might want to either add logic to check for missing keys, or process the post array before passing it.