Click to See Complete Forum and Search --> : unable to close mysql between functions


Dopple
12-27-2006, 07:06 AM
Can anyone tell me how I would close my mysql connection in the setup below?
I have a connectDB function which is called in the newUser function but I cannot close the connection in newUser using mysql_close($link).
I get that this is to do with the scope of the variable but can anyone tell me how I would acheive this?
Thanks
Graham

function connectDB(){
include( 'config.php' );
$link = mysql_connect($host, $user, $pass);
if (!$link) {
die('<p>Could not connect: ' . mysql_error() . '</p>');
}
mysql_select_db($db) or die('<p>Could not select database</p>');
}

function newUser($user_name, $user_password, $user_password2, $user_forename, $user_surname, $user_email, $user_avatar, $user_about){
$user_password = sha1($user_password);
connectDB();
$query = "INSERT INTO a_user VALUES ('', '$user_name', '$user_password', '$user_forename', '$user_surname', '$user_email', 'x', 0, 0, '$user_avatar', NOW(), 0, '$user_about')";
$result = mysql_query($query) or die('<p>Query failed: ' . mysql_error() . '</p>');
mysql_close($link);
}

NightShift58
12-27-2006, 07:40 AM
You'll have to define $link as global in your newuser()function, using the following statement:global $link;

Dopple
12-27-2006, 07:49 AM
Thanks. It didn't work when I juist used it in newUser but when i put it also in connectDB it worked fine.
Thank you.

NightShift58
12-27-2006, 07:58 AM
Correct. My mistake. It has to be defined global everywhere under the main script.

NogDog
12-27-2006, 11:59 AM
Alternatively, you might want to return the link from the one function and pass it as a parameter to the other. (I prefer to avoid the use of "global" whenever possible, as it has an annoying tendency to create hard-to-find bugs as scripts evolve and parts get re-used in other scripts.)

function connectDB(){
include( 'config.php' );
$link = mysql_connect($host, $user, $pass);
if (!$link) {
die('<p>Could not connect: ' . mysql_error() . '</p>');
}
mysql_select_db($db) or die('<p>Could not select database</p>');
return($link);
}

function newUser($user_name, $user_password, $user_password2, $user_forename, $user_surname,
$user_email, $user_avatar, $user_about, $link){
$user_password = sha1($user_password);
connectDB();
$query = "INSERT INTO a_user VALUES ('', '$user_name', '$user_password', '$user_forename', '$user_surname', '$user_email', 'x', 0, 0, '$user_avatar', NOW(), 0, '$user_about')";
$result = mysql_query($query) or die('<p>Query failed: ' . mysql_error() . '</p>');
mysql_close($link);
}

NightShift58
12-27-2006, 12:26 PM
You're absolutely right, it's best to avoid "global" for exactly the reasons you mention.

I didn't dare "read more into this" because I got hit hard a few days ago - which led to a not very impartial warning - so I'm playing it safe.

But, taking your cue, let me dare... I question whether there's a real need to do a mysql_connect() and mysql_close() before each insert. The insert has been placed in a function, which tends to indicate intent for repeated use within a script and/or application. Depending on the numbers, this culd lead to a huge and unecessary overhead. In a "standard" setting, I would connect once and - maybe, probably - explicitly close once.

NogDog
12-27-2006, 12:40 PM
You're absolutely right, it's best to avoid "global" for exactly the reasons you mention.

I didn't dare "read more into this" because I got hit hard a few days ago - which led to a not very impartial warning - so I'm playing it safe.

But, taking your cue, let me dare... I question whether there's a real need to do a mysql_connect() and mysql_close() before each insert. The insert has been placed in a function, which tends to indicate intent for repeated use within a script and/or application. Depending on the numbers, this culd lead to a huge and unecessary overhead. In a "standard" setting, I would connect once and - maybe, probably - explicitly close once.
Certainly a point worth considering. If there's a possibility that those functions will be used iteratively within one script, it would probably be more efficient to only connect once. Alternatively, you could just remove the mysql_close() from the function. Then any repeated calls to mysql_connect() which use the same host/user/password parameters will re-use the existing connection (as long as you don't specify TRUE for the optional 4th, "new link" parameter). Still a bit of extra processing, but probably not as time consuming as actually opening and closing the MySQL connection that many times.

Dopple
12-28-2006, 04:07 AM
Thanks for your thoughts on this guys.
This project I'm working on at the moment is something I'm doing just to get a better understanding of PHP and creating my own functions. Once I've finished this I'll look into optimising my code and probably then look into using classes. I've been dragging my arse along the ground when it comes to PHP. I think now's the time (since it's so quiet at work) to really give myself a push. I will look into my functions there and take on board your comments.

Thanks a bunch guys.
Graham