Improving this simple class...

[cgm225]

I am VERY new to OOP, and wanted to get some feedback on my first class.  I want to do this the right way.  Any feedback on naming conventions, syntax, making it more efficient, improving it for debugging etc. would be greatly appreciated!

 

class mysql {
    private $connection;   
    protected $result;     
    protected $sql;

    public function Connect($host, $name, $pass, $db) {
        $this->connection = mysql_connect($host,$name,$pass);
        mysql_select_db($db, $this->connection);        
    }

    public function Close(){
        mysql_close($this->connection);
    }

    public function Query($sql) {
        $this->sql = $sql;
        $this->result = mysql_query($this->sql,$this->connection) or die(mysql_error());
    }

    public function FetchArray() {
        return mysql_fetch_array($this->result);
    }

    public function FetchNum(){
        return mysql_num_rows($this->result);
    }
}

 

Thanks again!

[Naez]

This is a great start, very commendable.

 

What I've done with my DB class is I've extended it to encompass more than just MySQL connections.  I also have it build array for me

 

(For instance I'll do)

<?php
abstract class dbconn
{	

protected function __construct()
{
}

/* getInstance should be abstract but will output a PHP strict warning if it is.
// the following is code that should be used in this context:
abstract static public function getInstance(){
	if(is_null(self::$instance)){
		self::$instance = new self();
	}
	return self::$instance;
} */
abstract public function connect($dbhost,$dbuser,$dbpass,$dbname=NULL);
abstract public function select_db($dbname);
abstract public function sql_query($query);
abstract public function fetch_array($res=NULL);
abstract protected function sql_escape($msg);
abstract protected function sql_close();

static public function factory($type)
{
	switch ($type)
	{
		case "mysql":
			return mysql::getInstance();
		break;
		case "postgresql":
			return postgresql::getInstance();
		break;
		default:
		break;
	}
}	

public function sql_validate_value($var)
{
	if (is_null($var))
	{
		return 'NULL';
	}
	else if (is_string($var))
	{
		return "'" . $this->sql_escape($var) . "'";
	}
	else
	{
		return (is_bool($var)) ? intval($var) : $var;
	}
}


public function sql_build_array($query, $assoc_ary = false)
{
	if (!is_array($assoc_ary))
	{
		return false;
	}

	$fields = $values = array();

	if ($query == 'INSERT' || $query == 'INSERT_SELECT')
	{
		foreach ($assoc_ary as $key => $var)
		{
			$fields[] = $key;

			if (is_array($var) && is_string($var[0]))
			{
				// This is used for INSERT_SELECT(s)
				$values[] = $var[0];
			}
			else
			{
				$values[] = $this->sql_validate_value($var);
			}
		}

		$query = ($query == 'INSERT') ? ' (' . implode(', ', $fields) . ') VALUES (' . implode(', ', $values) . ')' : ' (' . implode(', ', $fields) . ') SELECT ' . implode(', ', $values) . ' ';
	}
	else if ($query == 'MULTI_INSERT')
	{
		$ary = array();
		foreach ($assoc_ary as $id => $sql_ary)
		{
			// If by accident the sql array is only one-dimensional we build a normal insert statement
			if (!is_array($sql_ary))
			{
				return $this->sql_build_array('INSERT', $assoc_ary);
			}

			$values = array();
			foreach ($sql_ary as $key => $var)
			{
				$values[] = $this->sql_validate_value($var);
			}
			$ary[] = '(' . implode(', ', $values) . ')';
		}

		$query = ' (' . implode(', ', array_keys($assoc_ary[0])) . ') VALUES ' . implode(', ', $ary);
	}
	else if ($query == 'UPDATE' || $query == 'SELECT')
	{
		$values = array();
		foreach ($assoc_ary as $key => $var)
		{
			$values[] = "$key = " . $this->sql_validate_value($var);
		}
		$query = implode(($query == 'UPDATE') ? ', ' : ' AND ', $values);
	}

	return $query;
}	


}
?>

 

Then I have a mysql connection that extends off of that.

<?php
class mysql extends dbconn
{
static private $instance = null;
private $_conn;
private $_db;
public $num_queries;
public $result;

protected function __construct(){
}

static public function getInstance(){
	if(is_null(self::$instance)){
		self::$instance = new self();
	}
	return self::$instance;
}

public function connect($dbhost,$dbuser,$dbpass,$dbname=NULL)
{
	$this->_conn = @mysql_connect($dbhost,$dbuser,$dbpass) or die('Could not connect: ' . mysql_error());
	if ($dbname){	$this->select_db($dbname); }
}

public function select_db($dbname)
{
	$this->_db = @mysql_select_db($dbname, $this->_conn) or die('Could not select specified database: ' . mysql_error());
}

public function sql_query($query)
{
	if (!$query)
	{	
		return false;	
	}
	else
	{
		$this->num_queries++;
		$result = @mysql_query($query, $this->_conn) or die('Query was not valid: ' . mysql_error());
		$this->result = $result;
		return $result;
	}
}

//  fetch_array from a result
public function fetch_array($res=NUlL)
{
	if ($res){ $this->result = $res; }
	return @mysql_fetch_assoc($this->result);
}

// make our strings going in safe
protected function sql_escape($msg)
{
	return @mysql_real_escape_string($msg);
}

// Database Connection closer
protected function sql_close()
{
	$this->_db = '';
	@mysql_close($this->_conn);
}

// allows ability to query a file that holds SQL syntax... great for building a table
public function mysql_query_file($url, $ignoreerrors = false) 
{
	$file_content = file($url);
	$query = "";

	foreach($file_content as $sql_line) 
	{
		$tsl = trim($sql_line);

		if (($sql_line != "") && (substr($tsl, 0, 2) != "--") && (substr($tsl, 0, 1) != "#")) 
		{
			$query .= $sql_line;

			if(preg_match("/;\s*$/", $sql_line)) 
			{
				$query = str_replace(";", "", "$query");
				$result = mysql_query($query);

				if (!$result && !$ignoreerrors) die(mysql_error());
				$query = "";
		   	}
		}
	}
}

}
?>

 

So when I call it I do.

 

<?php
$db = $dbconn->factory("mysql");
$db->connect($dbhost,$dbuser,$dbpass,$dbname)

$selectarray = array(
                     'id' = $_GET['id'],
                    'sessionid' = $_SESSION['id']
                       );
$db->sql_query("SELECT * FROM users " . $db->sql_build_array("SELECT", $selectarray) . " LIMIT 1")

$row = $db->fetch_array($db->result))

echo $row['username']

?>

 

 

 

Here's the thread with mine.

 

http://www.phpfreaks.com/forums/index.php/topic,186452.0.html

[448191]

Syntax: use UpperCamelCase for class names and lowerCamelCase for class members (properties and methods).

 

Debugging: don't ever, ever, call die() on an error. At the very least use trigger_error(), but preferably you throw a typed exception (such as MySQLException).

[cgm225]

Thank you both for your help.  I am really trying to understand those classes.  However, I am getting some syntax errors with the code you included for calling those classes::

 

 

<?php
$db = $dbconn->factory("mysql");
$db->connect($dbhost,$dbuser,$dbpass,$dbname)

$selectarray = array(
                     'id' = $_GET['id'],
                    'sessionid' = $_SESSION['id']
                       );
$db->sql_query("SELECT * FROM users " . $db->sql_build_array("SELECT", $selectarray) . " LIMIT 1")

$row = $db->fetch_array($db->result))

echo $row['username']

?>

 

Any ideas?

 

Also, (and this may be more directed towards 448191), is this a good class to start with in transitioning my gallery script over to OOP?  See::  http://www.phpfreaks.com/forums/index.php/topic,187082.0.html

 

[448191]

It's a start for encapsulating database logic, but your script does more than that. It handles input and decides what to do with it. Start with a basic Front Controller (use forum search).

[staar2]

hm i would to the fetching data into other class so it could be easily extend able.