Page 1 of 2

Am I reinventing the wheel?

Posted: Mon Oct 01, 2007 9:39 am
by ctown82
Hi, first post here, by the way. o/

I'm a PHP newbie, and started with the tutorials on php-mysql-tutorial.com, and since bought O'Reilly's "Learning PHP & MySQL" to further my edumacation. I really like the language, and coming from trying to learn C and C++ on my own (a little difficult, I might add), I'm finding PHP pretty easy so far.

My problem, being new to this, is that I'm ignorant of many functions some vets wouldn't even think about while they utilized them. That said, I'm presently working on a simple DB script that connects to a database, and submits a supplied query.

So, my main question is: Am I reinventing the wheel? Also: Is there something else I could do to improve this?

Here's what I've got so far...

Code: Select all

<?php

/**
 * Filename: db_login.php
 */

define('db_hostname', 'localhost');
define('db_database', 'dbname');
define('db_username', 'username');
define('db_password', 'password');

// 0 = error silently, 1 = simple errors, 2 = verbose errors.
$db_error_level = 0;

?>

Code: Select all

<?php

/**
 * Filename: sql.php
 */

require_once('db_login.php');

/**
 * Fx sendQ(SQL query)
 * Submits query and returns SQL resource, then closes the DB connection.
 * Will output mysql_error() reports if db_verbose_errors (db_login.php) is true, which
 * a security risk, since it displays important DB information.
 * Will output simple errors if db_silent_errors (db_login.php) is false.
 * NOTE: db_verbose_errors is checked before db_silent_errors, so if verbose AND silent
 * errors are enabled, errors will be printed verbosely.
 * 
 * @param string $query
 * @return SQL_Resource
 */
function sendQ($query)
{
	global $db_verbose_errors;
	
	$connection = @mysql_connect(db_hostname, db_username, db_password);
	if(!$connection)
	{
		qError("Could not connect to the SQL host.");
	}
	else
	{
		if(!(@mysql_select_db(db_database, $connection)))
		{
			qError("Could not select the database.");
		}
		else
		{
			if(!($result = @mysql_query($query)))
			{
				qError("Could not execute query.");
			}
		}
	}
	@mysql_close($connection);
	return $result;
}

/**
 * Fx qError($arg)
 * Prints a SQL error. Verboseness depends on $db_error_level set in (db_login.php).
 * 
 * @param string $arg
 * @return none
 */
function qError($arg)
{
	global $db_error_level;

	switch($db_error_level)
	{
		case 0:
			die();
			break;
		case 1:
			die("$arg <br />". mysql_error());
			break;
		case 2:
			die("$arg <br />");
			break;
		default:
			break;
	}
}

?>

Posted: Mon Oct 01, 2007 9:51 am
by feyd
Connecting and disconnecting from MySQL all the time probably isn't the best of ideas, especially since the resource depends on the connection to retrieve data.

Also, I suggest reading: viewtopic.php?t=55714

Posted: Mon Oct 01, 2007 10:09 am
by ctown82
So constants are slower... Okay, that'll be something I think I'll change then... No reason to use them if I'm not benefiting from them. (I thought they'd be faster.)

I thought that keeping a persistant connection was a bad thing? I can see why having a connection open for more than one query would give a performance boost, maybe I can work it into the function as an argument or something.

Like, if I were to do 2 or 3 different queries using something like this...

sendQ("SELECT * FROM aTable", true);
sendQ("UPDATE ... ... ...", true);
sendQ("SELECT * FROM aTable", false);

Code: Select all

<?php

/**
 * Filename: sql.php
 */

require_once('db_login.php');

/**
 * Fx sendQ(SQL query)
 * Submits query and returns SQL resource, then closes the DB connection.
 * 
 * @param string $query
 * @param boolean $keep_conn_alive
 * @return SQL_Resource
 */
function sendQ($query, $keep_conn_alive = false)
{
	global $db_verbose_errors;
	
	$connection = @mysql_connect(db_hostname, db_username, db_password);
	if(!$connection)
	{
		qError("Could not connect to the SQL host.");
	}
	else
	{
		if(!(@mysql_select_db(db_database, $connection)))
		{
			qError("Could not select the database.");
		}
		else
		{
			if(!($result = @mysql_query($query)))
			{
				qError("Could not execute query.");
			}
		}
	}
	if(!$keep_conn_alive)
	{
		@mysql_close($connection);
	}
	return $result;
}

/**
 * Fx qError($arg)
 * Prints a SQL error. Verboseness depends on $db_error_level set in (db_login.php).
 * 
 * @param string $arg
 * @return none
 */
function qError($arg)
{
	global $db_error_level;

	switch($db_error_level)
	{
		case 0:
			die();
			break;
		case 1:
			die("$arg <br />". mysql_error());
			break;
		case 2:
			die("$arg <br />");
			break;
		default:
			break;
	}
}

?>
The only problem I see is my needing to be careful to close the connection on the last query.

Posted: Mon Oct 01, 2007 10:18 am
by Zoxive
ctown82 wrote:The only problem I see is my needing to be careful to close the connection on the last query.
PHP does do auto clean up, but it is a better practice to do it yourself.

I use a Class for my Database Connections, and then on __destruct have it close the connection.

Edit: btw, It is a Singleton class, that I use.

Posted: Mon Oct 01, 2007 10:19 am
by s.dot
I suppose your functions are meant to handle the error handling itself.. but I'm sure it'd be better off if you remove the error suppressors (@) and let PHP spit out notices/warnings all over the place on errors.

If anything it'll get you in the practice of writing good code.

Posted: Mon Oct 01, 2007 10:32 am
by ctown82
Zoxive wrote:
ctown82 wrote:The only problem I see is my needing to be careful to close the connection on the last query.
PHP does do auto clean up, but it is a better practice to do it yourself.

I use a Class for my Database Connections, and then on __destruct have it close the connection.
I've played with classes a bit, but they keep me awake at night... in the fetal position... sobbing uncontrollably.

I guess that means I should spend more time with them. :P
scottayy wrote:If anything it'll get you in the practice of writing good code.
How so? What's considered 'bad code' in a suppressed error?




EDIT: Thanks for all the replies, BTW. :)

Posted: Mon Oct 01, 2007 10:35 am
by feyd
ctown82 wrote:How so? What's considered 'bad code' in a suppressed error?
Suppressing the error.

Posted: Mon Oct 01, 2007 10:41 am
by ctown82
:P

So I think the question now is: Is it their fault for giving me the gun that I shot myself in the foot with, or is it my fault for cleaning it while it was loaded?

Posted: Mon Oct 01, 2007 10:45 am
by s.dot
You're on the right track man ;) This forum is the best place for people who get guided wrong by articles or tutorials (not saying the one you found is right or wrong).

Also, a database connection gets closed at the end of script execution (unless you use a persistent connection). There is no need to implicitly close the connection (although it can be done, and in some cases needs to be done).

Posted: Mon Oct 01, 2007 12:47 pm
by RobertGonzalez
You should close your own connetions unless you are using persistent connections. Also I'd recommend opening your connection early in the app and closing at the very end. Keeping the connection for the entire 0.00034 seconds your script is running is really not going to hog a ton of resource unless your app is serving up tens of thousands of requests or more. Another thing I would add is that if you are not distributing this app, there is really no need to read your database connection details into constants or variables. Just put them into the one database connection function at the opening of the script and you are golden. Plus there are not vars hanging around that could be glommed on to my some malicious piece of code looking to include scripts from unprotected servers.

As for the error suppressing, typically that is considered bad practise. Ideally you want to catch an errors that you might run across rather than shut them up.

Posted: Mon Oct 01, 2007 1:26 pm
by Kieran Huggins
Wow - do people actually close mySQL connections? I started doing it years ago and quickly stopped bothering.

I have since switched to persistent connections and am forced to wonder why you would ever want to close a connection?

Thoughts?

Posted: Mon Oct 01, 2007 1:39 pm
by s.dot
http://www.php.net/mysql_close wrote:Using mysql_close() isn't usually necessary, as non-persistent open links are automatically closed at the end of the script's execution. See also freeing resources.

Posted: Mon Oct 01, 2007 1:39 pm
by Zoxive
Kieran Huggins wrote:Wow - do people actually close mySQL connections? I started doing it years ago and quickly stopped bothering.

I have since switched to persistent connections and am forced to wonder why you would ever want to close a connection?

Thoughts?
Depending on the Application.

Persistent Connections take longer to start.

Posted: Mon Oct 01, 2007 3:32 pm
by mrkite
I always explicitly close the connection as soon as I'm done with it.

You just never know what's going to happen. Perhaps later in the script I file_get_contents on something that's down... and it takes 30 seconds to timeout. That's a long time to hold open a mysql connection for no reason other than you were too damn lazy to close it when you were done with it.

Posted: Mon Oct 01, 2007 10:57 pm
by s.dot
Kieran Huggins wrote:Wow - do people actually close mySQL connections? I started doing it years ago and quickly stopped bothering.

I have since switched to persistent connections and am forced to wonder why you would ever want to close a connection?

Thoughts?
The majority of a "page view" is not getting information from the database. It's the end user reading the content, looking at images, writing responses, etc. Do you really want the database connection open that long? Especially if hundreds of them are concurrent.