Page 1 of 3
My Mysql Class - Advice
Posted: Mon Jul 24, 2006 9:38 pm
by Luke
Every time somebody posts their mysql interaction class on this forum (and it seems often) the response is... keep it minimal. I am wondering if this is too minimal... am I missing something... missing the point? Please nitpick!
Code: Select all
<?php
class Mysql{
private $host;
private $user;
private $pass;
private $db;
private $registry;
private $config;
function __construct($registry, $host=false, $user=false, $pass=false, $db=false, $persistant=true){
$this->registry = $registry;
$this->config = $this->registry->get('Config');
$this->error = $this->registry->get('Error');
//$this->error->set_source(get_class($this));
$this->host = $host ? $host : $this->config->get('host');
$this->user = $user ? $user : $this->config->get('user');
$this->pass = $pass ? $pass : $this->config->get('pass');
$this->db = $db ? $db : $this->config->get('db');
$this->persistant = $persistant;
}
public function connect($select_db=true){
$function = $this->persistant ? 'mysql_pconnect' : 'mysql_connect';
$function($this->host, $this->user, $this->pass)
or $this->error->push('Connection failed:\n' . mysql_error());
if($select_db){
$this->select_db();
}
}
public function select_db($db=false){
if($db) $this->db = $db;
mysql_select_db($this->db);
}
public function execute($query){
$this->connect();
if($r = mysql_query($query)){
return mysql_affected_rows();
}
else{
$this->error->push('SQL Execution Failed:\n' . mysql_error());
}
}
// TODO: get this working
public function query($query, $array_type='default'){
$this->connect();
switch($array_type){
case "MYSQL_ASSOC":
$set_array_type = MYSQL_ASSOC;
break;
case "MYSQL_NUM":
$set_array_type = MYSQL_NUM;
break;
default:
$set_array_type = MYSQL_BOTH;
}
if($r = mysql_query($query)){
return mysql_fetch_array($r, $set_array_type);
}
else{
$this->error->push('SQL Query Failed:\n' . mysql_error());
}
}
}
?>
Posted: Mon Jul 24, 2006 9:42 pm
by Benjamin
Looks ok. I've found that the database connection code should be in a seperate class, that way you can run more than one instance using the same database connection. The db class I wrote has a lot of bloat in it, but I like it because it saves sooo much time when I do long inserts and selects.
Posted: Mon Jul 24, 2006 11:59 pm
by Benjamin
If you use mysql_fetch_assoc, you won't have to specify whether you want associative or indexed values as both will be returned..
Code: Select all
public function query($query){
$this->connect();
if($r = mysql_query($query)){
return mysql_fetch_assoc($r);
}
else{
$this->error->push('SQL Query Failed:\n' . mysql_error());
}
}
That function doesn't make much sense though, because you'll have to query the db for every row..
You could just return every row in an array..
Code: Select all
<?php
public function query($query){
$this->connect();
if($r = mysql_query($query)){
$rCount = mysql_num_rows($r);
$records = array();
if ($rCount > 0)
{
for ($i = 0; $i < $rCount; $i++;)
{
$records[$i] = mysql_fetch_assoc($r);
}
return $records;
} else {
return false;
}
} else {
$this->error->push('SQL Query Failed:\n' . mysql_error());
}
}
?>
Posted: Tue Jul 25, 2006 1:52 am
by Luke

thanks... I hadn't even tested out that query method before I posted it... and I like the idea of using a seperate class for connection... I am working on that now. Any suggestions for methods in the class other than the ones there though? (I am going to add a close method, but anything else?)
Posted: Tue Jul 25, 2006 1:59 am
by Benjamin
The Ninja Space Goat wrote::Any suggestions for methods in the class other than the ones there though?
I'm probably the wrong person to ask, I would load the thing up lol..
viewtopic.php?t=51623
Posted: Tue Jul 25, 2006 3:09 am
by Jenk
My *sql classes contain an iterator for the results (results are stored in an array - which I use an object for as well, but essentially it's just the same as normal array as per below):
Code: Select all
public function fetchRow()
{
if (isset($this->result[$this->index])) {
return $this->result[$this->index++];
} else {
$this->index = 0;
return false;
}
}
I've also exapanded on that in the past to use optional arg $index to specificy which row to return. Can also store multiple results if need be too.
The above adds the ability to while() loop the results just as if using mysql_fetch_*:
Code: Select all
while ($row = $db->fetchRow()) {
//do something.
}
Posted: Tue Jul 25, 2006 12:06 pm
by pickle
I'd say get rid of the connect() and select_db() functions.
You're never going to be instantiating this object without wanting to connect to the database, so why not put that in the constructor?
The same applies to the select_db() function. You're passing along the DB to the constructor, why not just connect & select the DB there?
Posted: Tue Jul 25, 2006 12:44 pm
by jayshields
Put your \n's in double quotes!!

Posted: Tue Jul 25, 2006 1:04 pm
by Luke
jayshields wrote:Put your \n's in double quotes!!

d'oh! Thanks.
Posted: Tue Jul 25, 2006 2:09 pm
by Chris Corbyn
astions wrote:Looks ok. I've found that the database connection code should be in a seperate class, that way you can run more than one instance using the same database connection. The db class I wrote has a lot of bloat in it, but I like it because it saves sooo much time when I do long inserts and selects.
I think that may be a bit of an overkill. Perhaps provide a method of fetching a singleton instead.
Posted: Tue Jul 25, 2006 3:35 pm
by daedalus__
i just make my db class return resources.
Posted: Tue Jul 25, 2006 3:39 pm
by Luke
this class is going to be used strictly by other (more specific) classes... so the point of it is to:
1. Connect to the database (but only if a query is ran... not auto-connect, which is why mysql_connect isn't in the constructor)
2. Run (select) queries and return the results in a neat format
3. Execute (update, delete, etc.) queries and return whether or not it was successful
That's basically it. I don't want it more complicated than that.
Posted: Tue Jul 25, 2006 3:47 pm
by John Cartwright
The Ninja Space Goat wrote:this class is going to be used strictly by other (more specific) classes... so the point of it is to:
1. Connect to the database (but only if a query is ran... not auto-connect, which is why mysql_connect isn't in the constructor)
2. Run (select) queries and return the results in a neat format
3. Execute (update, delete, etc.) queries and return whether or not it was successful
That's basically it. I don't want it more complicated than that.
Why not establish the connection once, and use d11's suggestion of adding a method to return the instance of the class?
Posted: Tue Jul 25, 2006 3:52 pm
by Luke
I have tried to create a singleton before and failed (I am doing something wrong, and I Can't post my code right now because it's at home). I like the idea though... I will work on doing it that way. Then I need to somehow get my registry to accept singletons.
Posted: Tue Jul 25, 2006 5:21 pm
by Chris Corbyn
The Ninja Space Goat wrote:I have tried to create a singleton before and failed (I am doing something wrong, and I Can't post my code right now because it's at home). I like the idea though... I will work on doing it that way. Then I need to somehow get my registry to accept singletons.
Your registry shouldn't care if the object is a singleton.... it's still an object. Then again, I'm not sure I've seen how you've written your registry. Is it a factory as well as a registry or something?
As for allowing a singleton to be fetched it's just a case of adding a static property to the class and a static method to return that. Think of static properties as encapsulated globals (which is why some people don't like them!) in this scenario.
Code: Select all
class MySQL
{
static private $instance = null;
/* snippety snip... */
static public function getInstance($registry, $host=false, $user=false, $pass=false, $db=false, $persistant=true)
{
if (self::$instance === null)
{
self::$instance = new MySQL($registry, $host, $user, $pass, $db, $persistant);
}
return self::$instance;
}
Then you can either use your class like it is now or you can use a singleton instance by doing this rather than instantiating it:
Code: Select all
$db = MySQL::getInstance($reg, 'localhost');