[SOLVED] Help with function

[Andy-H]

OK, I don't have this script running, I was just curious to weather it would work because I may use it in future projects if it does. Also if you see any bad practice in the code please let me know.

 

Ok, so imagine this file is "req/functions.inc" and there is a db connection file called "req/db_conn.inc".

 

<?php

require_once("db_conn.inc");

$username = (string)mysql_real_escape_string($_SESSION['username']);

function userVars($vars){

global $username;

	if ( !is_array($vars) ){
		echo $vars . ' - Invalid input for function(userVars), please use arrays only...';
	}else{

$num = (int)count($vars);

	for ($i = 0; $i < $num; $i++){
		$string .= $vars[$i] . " , ";
	}

       $_query    = "SELECT " . $string . " FROM users WHERE username = '" . $username . "' LIMIT 1";
	   $_result   = mysql_query($_query)or trigger_error('Query failed in function(userVars).');
global $_userVars = mysql_fetch_row($_result);

	}
}

?>

 

And this is some file I need to fetch the users info in...

 

<?php

require_once("req/functions.inc");

$_vars = array("username", "password", "email", "money", "status");

userVars($_vars);

$realname = $_uservars[0];
$pass     = $_userVars[1];
$email    = $_userVars[2];
$money    = $_userVars[3];
$status   = $_userVars[4];

?>

 

 

[Andy-H]

I did have the $_query and $_result variables declared as global scope originally but wasnt sure if this was neccessary and removed them.  ???

[akitchin]

it's not really necessary to define the username as global either - since you're using it from the SESSION superglobal, it's universally available anyway.  i'm also confused why you bother typecasting the returns from mysql_real_escape_string() and count() when PHP returns values with those types.

 

implode() would be more efficient than iterating and concatenating the array input yourself.

 

IMO you should simply be returning the array of information - i'm not sure why you'd put it into a global variable.  furthermore, leaving it as a numerically-keyed array doesn't really help, nor does extracting into local variables.

 

i mean, it looks pretty and stuff, but functionally, there's a lot of wasted typing.

[Andy-H]

<?php

require_once("db_conn.inc");

$username = mysql_real_escape_string($_SESSION['username']);

function userVars($vars){

	if ( !is_array($vars) ){
		trigger_error( $vars . ' - Invalid input for function(userVars), please use arrays only...' );
	}else{

	if ( count($vars) == 0 ){
		trigger_error( $vars . ' - Array empty, non-usable for function(userVars).');
	}else{

           $string = implode(",", $vars);

           $_query    = "SELECT " . $string . " FROM users WHERE username = '" . $username . "' LIMIT 1";
           $_result   = mysql_query($_query)or trigger_error('Query failed in function(userVars).');
global $_userVars = mysql_fetch_row($_result);

	}
	}
}

?>

 

Any better???

 

////////////////////////////////////////////

IMO you should simply be returning the array of information - i'm not sure why you'd put it into a global variable.  furthermore, leaving it as a numerically-keyed array doesn't really help, nor does extracting into local variables.

///////////////////////////////////////////

 

 

I dont really know what you mean by that? :S I wrote the function because I'm lazy and I though it would be an easier way to run my querys that I would use alot. With them being numerically keyed, the array would be defined in the script which the function is used in so it wouldnt be so hard to refer to...

 

[akitchin]

what i mean by returning the array is changing this line:

 

global $_userVars = mysql_fetch_row($_result);

 

to:

 

return mysql_fetch_row($_result);

 

and calling it as:

 

$_userVars = userVars($_vars);

 

the added advantage to using an associative array is that you can simply use extract() to pull those variables into the local namespace:

 

return mysql_fetch_assoc($_result);

 

and then:

 

$_userVars = userVars($_vars);
extract($_userVars);

 

it may seem silly that i'm just moving the $_userVars definition to outside the function, but again, the advantage here is that you aren't chained to that array name.

[Andy-H]

I see (said the blind man who couldn't see at all), lol. I took your advice on the return part and am looking into the extract function (after I go for a smoke lol) as I have never heard of it. Thanks for the help =D