Database wrapper class

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Database wrapper class

Post by s.dot »

I'm writing this for my own experience. I'm not done with it, by far, but I want to post what I have to see if you guys can offer suggestions.

My input:
I feel there needs to be more interaction within the class. I feel like I'm just renaming php functions. I'm just not sure how to interact.

And, as of now, this is for PHP4.

Code: Select all

<?php

/*
* MySQL database wrapper class
*/

class db
{
	/*
	* @param resource $link
	*/
	var $link_id;
	
	/*
	* Connects to a mysql database server
	* @param str $host
	* @param str $username
	* @param str $password
	*/
	function connect($host, $username, $password)
	{
		$this->link_id = mysql_connect($host, $username, $password) or die(mysql_error());
	}
	
	/*
	* Selects a database to use on database server
	* @param str $database
	*/
	function use_db($database)
	{
		mysql_select_db($database, $this->link_id);
	}
	
	/*
	* Performs a query on the database and returns the result set
	* @param str $sql
	*/
	function query($sql)
	{
		$result = mysql_query($sql, $this->link_id) or die(mysql_error());
        return $result;
	}
	
	/*
	* Gathers and returns the number of rows from a result set
	* @param resource $result
	*/
	function num_rows($result)
	{
		return mysql_num_rows($result);
	}
	
	/*
	* Escapes a variable for passing to a query
	* @param var $variable
	*/
	function escape($variable)
	{
		return mysql_real_escape_string($variable, $link=$this->link_id);
	}
	
	/*
	* Fetches a single row from a result set
	* @param resource $result
	* @param constant $fetch_type
	*/
	function fetch_row($result, $fetch_type=MYSQL_ASSOC)
	{
		return mysql_fetch_array($result, $fetch_type);
	}
	
}

?>
This is an example of how i'd be using it (proceedurally, for now). Anything that would cut down on the amount of coding needed in scripts, i'd like to implement into the class.

Code: Select all

if(empty($_GET['blog_id']) || !is_numeric($_GET['blog_id']))
{
	$error = 'Invalid Blog Entry Specified.';
} else
{
	$blog_id	= $db->escape(htmlentities(stripslashes($_GET['blog_id']), ENT_QUOTES));
	$result		= $db->query("SELECT * FROM `blogs` WHERE `id` = '$blog_id' LIMIT 1");
	
	if($db->num_rows($result))
	{
		$blog = $db->fetch_row($result);
	} else
	{
		$error = 'Invalid Blog Entry Specified';
	}
}
For example, when doing a query, should I save the result set, mysql_insert_id(), or anything else in the properties?

Also, I don't want input on what I should have in there (as i haven't gotten very far yet). Just input on what I have so far.

Thank you!
Last edited by s.dot on Sun May 27, 2007 4:59 pm, edited 1 time in total.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

Make query() check if $link_id exists before using it. Otherwise, call connect().

This makes it so that you can send this object between sessions, and maintain it's connection in the object (as it's lost from one page to the next).
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

The way I do web sites, I save the database connection script in db_connect.php, and include it in my header.php file, so it will be called on every page.

So for the time being, connect() will be called on every page.

If I were to call the connect() function from query() if no link_id existed, would I have to make $username, $password, and $host properties of the class instead of parameters of the connect() method?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

A few things off the top of my head
  • prepared statements
  • data caching
  • refactoring into more classes
  • multiple query support
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

scottayy wrote:The way I do web sites, I save the database connection script in db_connect.php, and include it in my header.php file, so it will be called on every page.

So for the time being, connect() will be called on every page.

If I were to call the connect() function from query() if no link_id existed, would I have to make $username, $password, and $host properties of the class instead of parameters of the connect() method?
Yes and no. Yes if you plan on giving this class out for public use.. No if it's just for you. Just give the parameters default values or something. At least, that's what I do.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

feyd wrote:A few things off the top of my head
  • prepared statements What's this? http://en.wikipedia.org/wiki/Prepare_%28SQL%29 ?
  • data caching By storing the result of query() in a property?
  • refactoring into more classes What I already have could be refactored?
  • multiple query support This would be a *to-do* item. I wanted input on what I had so far.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

I changed the connect() parameters to properties, and took superdesignz suggestion of checking for a link_id in query().

Code: Select all

<?php

/*
* MySQL database wrapper class
*/

class db
{
	/*
	* MySQL link identifier
	* @resource $link
	*/
	var $link_id;
	
	/*
	* MySQL host
	* @string $host
	*/
	var $host;
	
	/*
	* MySQL username
	* @string $username
	*/
	var $username;
	
	/*
	* MySQL password for $this->username
	* @string $password
	*/
	var $password;
	
	/*
	* Connects to a mysql database server
	*/
	function connect()
	{
		$this->link_id = mysql_connect($this->host, $this->username, $this->password)
			or die(mysql_error());
	}
	
	/*
	* Selects a database to use on database server
	* @param str $database
	*/
	function use_db($database)
	{
		mysql_select_db($database, $this->link_id);
	}
	
	/*
	* Performs a query on the database and returns the result set
	* @param str $sql
	*/
	function query($sql)
	{
		if(!$this->link_id)
		{
			$this->connect();
		}
		
		return mysql_query($sql, $this->link_id);
	}
	
	/*
	* Gathers and returns the number of rows from a result set
	* @param resource $result
	*/
	function num_rows($result)
	{
		return mysql_num_rows($result);
	}
	
	/*
	* Escapes a variable for passing to a query
	* @param var $variable
	*/
	function escape($variable)
	{
		return mysql_real_escape_string($variable, $this->link_id);
	}
	
	/*
	* Fetches a single row from a result set
	* @param resource $result
	* @param constant $fetch_type
	*/
	function fetch_row($result, $fetch_type=MYSQL_ASSOC)
	{
		return mysql_fetch_array($result, $fetch_type);
	}
	
}

?>
User avatar
Kieran Huggins
DevNet Master
Posts: 3635
Joined: Wed Dec 06, 2006 4:14 pm
Location: Toronto, Canada
Contact:

Post by Kieran Huggins »

isn't that what mysql_pconnect() is for?
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

... Maybe. :?

Does it hold the connection in $link_id when the object is serialized and saved in a session?
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

The next step is something like this:

Code: Select all

<?php

/*
* MySQL database wrapper class
*/

class Db_Connection
{
	/*
	* MySQL link identifier
	* @resource $link
	*/
	var $link_id;
	
	/*
	* MySQL host
	* @string $host
	*/
	var $host;
	
	/*
	* MySQL username
	* @string $username
	*/
	var $username;
	
	/*
	* MySQL password for $this->username
	* @string $password
	*/
	var $password;
	
	function __construct($host, $username, $password)
	{
		$this->host = $host;
		$this->username = $username;
		$this->password = $password;
	}
	
	/*
	* Connects to a mysql database server
	*/
	function connect()
	{
		$this->link_id = mysql_connect($this->host, $this->username, $this->password)
			or die(mysql_error());
	}
	
	/*
	* Selects a database to use on database server
	* @param str $database
	*/
	function select($database)
	{
		mysql_select_db($database, $this->link_id);
	}
	
	/*
	* Performs a query on the database and returns the result set
	* @param str $sql
	*/
	function query($sql)
	{
		if(!$this->link_id)
		{
			$this->connect();
		}
		
		$result = new Db_Result($this->link_id, mysql_query($sql, $this->link_id));
		return $result;
	}
	
	/*
	* Escapes a variable for passing to a query
	* @param var $variable
	*/
	function escape($variable)
	{
		return mysql_real_escape_string($variable, $this->link_id);
	}
}
	
class Db_Result
{
	/*
	* MySQL link identifier
	* @resource $link
	*/
	var $link_id;
	
	/*
	* MySQL result identifier
	* @resource $result
	*/
	var $result;
	
	function __construct($link, $result)
	{
		$this->link = $link;
		$this->result = $result;
	}
	
	/*
	* Fetches a single row from a result set
	* @param resource $result
	* @param constant $fetch_type
	*/
	function fetchArray()
	{
		return mysql_fetch_assoc($this->result);
	}
	
	/*
	* Fetches a single row from a result set
	* @param resource $result
	* @param constant $fetch_type
	*/
	function fetchObject()
	{
		return mysql_fetch_object($this->result);
	}
	
	/*
	* Gathers and returns the number of rows from a result set
	* @param resource $result
	*/
	function num_rows()
	{
		return mysql_num_rows($this->result);
	}
	
}

?>
You probably also want to add isError() and getError() methods, plus the stuff that feyd mentioned.
(#10850)
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Yeah I was gonna say taking the connection params off the connect call wasn't a good idea but arborint's design is better yet than the original. Alternatively you can have a connection factory that will only return a successfully connect object. The the credentials only need be there and not on the main object then...better cohesion.
feyd wrote:data caching
Would that be premature optimization you are suggesting their feyd? :P
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

Going to take me a while to digest that aborint.

Let me try to summarize what you did:

You refactored by separating the connection, and result operations into separate classes.
When instantiating a class, the __construct method is executed on instantiation?
is __construct PHP4 too?

Would I separate these classes into separate files? I would, but, then instead of just calling new class(), i'd have to require 'class.php', then do new().
Last edited by s.dot on Sun May 27, 2007 6:30 pm, edited 1 time in total.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

I see no Everah.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

__construct is PHP5.

And don't take it personally ole. Everah's avatar is cool too. Easy mix up.

Gir.. Penguin.. One in the same, right? :-p
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

Could've swore that was everah 8O My appologies, aborint!
Post Reply