Am I reinventing the wheel?

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

ctown82
Forum Newbie
Posts: 11
Joined: Mon Oct 01, 2007 9:29 am

Am I reinventing the wheel?

Post 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;
	}
}

?>
Last edited by ctown82 on Mon Oct 01, 2007 9:53 am, edited 1 time in total.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post 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
ctown82
Forum Newbie
Posts: 11
Joined: Mon Oct 01, 2007 9:29 am

Post 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.
User avatar
Zoxive
Forum Regular
Posts: 974
Joined: Fri Apr 01, 2005 4:37 pm
Location: Bay City, Michigan

Post 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.
Last edited by Zoxive on Mon Oct 01, 2007 10:22 am, edited 1 time in total.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post 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.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
ctown82
Forum Newbie
Posts: 11
Joined: Mon Oct 01, 2007 9:29 am

Post 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. :)
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

ctown82 wrote:How so? What's considered 'bad code' in a suppressed error?
Suppressing the error.
ctown82
Forum Newbie
Posts: 11
Joined: Mon Oct 01, 2007 9:29 am

Post 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?
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post 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).
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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.
User avatar
Kieran Huggins
DevNet Master
Posts: 3635
Joined: Wed Dec 06, 2006 4:14 pm
Location: Toronto, Canada
Contact:

Post 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?
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post 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.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
Zoxive
Forum Regular
Posts: 974
Joined: Fri Apr 01, 2005 4:37 pm
Location: Bay City, Michigan

Post 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.
mrkite
Forum Contributor
Posts: 104
Joined: Tue Sep 11, 2007 4:19 am

Post 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.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post 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.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
Post Reply