Page 1 of 1

Database Class - please comment

Posted: Tue Apr 12, 2005 9:29 pm
by SBro
Here is a database class I am working on, I'd appreciate any feedback you can give me. Should I now create another class which provides interaction with the database? ie. update/remove functions etc, or should I place these functions in this existing class? thanks.

Code: Select all

<?php

/********************
 * Copyright (c) 2005
 * db.class.php
 * Base database functions
 ********************/

// required files
require_once('config.php');

class db {
	
	// class variables
	var $results;
	var $db;
	var $host;
	var $user;
	var $pass;

	/**
	 * @return void
	 * @desc db constructor
	 **/
	function db() {
		$this->db = DB_DB;
		$this->user = DB_USER;
		$this->host = DB_HOST;
		$this->pass = DB_PASS;
		
		// connect to database
		$this->connect();
	}
	
	/**
	 * @return boolean
	 * @desc connects and selects database
	 **/
	function connect() {
		if (!@mysql_connect($this->host, $this->user, $this->pass)) {
			// can't connect to database
			error::log(ERR_MED, 'Unable to connect to '.$this->user.'@'.$this->host.':<br>'.mysql_error(), ERR_SHOW);
			return false;
		}
		if (!@mysql_select_db(DB_DB)) {
			// can't select database
			error::log(ERR_MED, 'Unable to select database '.$this->db.':<br>'.mysql_error(), ERR_SHOW);
			return false;
		}
		return true;
	}
	
	/**
	 * @return boolean
	 * @param string $query
	 * @desc performs selected query
	 **/
	function query($query) {
		if (!@mysql_query($query)) {
			error::log(ERR_MED, 'Unable to execute query:<br>'.mysql_error(), ERR_SHOW);
			return false;
		}
		return true;
	}
	
	/**
	 * @return array
	 * @param resource $result
	 * @param string $type
	 * @desc gets array of specified type
	 **/
	function fetch_array($result, $type = 'MYSQL_ASSOC') {
		// get results
		$this->results = @mysql_fetch_array($result, $type);
		if (!$this->results) {
			// can't fetch array
			error::log(ERR_MED, 'Unable to get result:<br>'.mysql_error(), ERR_SHOW);
		}
		return $this->results;
	}
	
	/**
	 * @return int
	 * @desc gives us number of rows
	 **/
	function num_of_rows() {
		$num = @mysql_num_rows($this->results);
		if (!$num) {
			return 0;
		}
		return $num;
	}
	
}

?>

Posted: Tue Apr 12, 2005 10:52 pm
by infolock
nice and simple. i'm guessing this is being used to work in a string of scripts you have written or are currently working with. As far as the inserts, i'd definately go that route.

the only question would be in reguards to this function :

Code: Select all

function fetch_array($result, $type = 'MYSQL_ASSOC') {
//...
are you only going to be using MYSQL_ASSOC, or are you gonna be going from both, to assoc, or even to none? If you are only gonna be using the assoc title (which i think i've always used), why not just use mysql_fetch_assoc? if however you are gonna be changing the type of array you want to return, then do disreguard this comment ;)

but yeah, again, i'd definatley just put my mysql insert/alter/update and all that in this class. that's what a class is for, is to horde all your objects ;) otherwise, ur just gonna waste directory space by creating a new class.. this class was aimed at being a mysql class right? so why chanage that :)

Posted: Tue Apr 12, 2005 11:00 pm
by SBro
Thanks for the response, yes fetch_array() will primarily being used in mysql_fetch_assoc form, the reason for adding the $type was simply to have the ability to change if required. As for the insert/update/delete etc functions, I was going to put them in a seperate class because I'm not sure how complex they will get, the project I'm working on is likely to have some pretty complex queries and that was my reason. Also in the reading I've done it seems though people seem to reccomend having a database abstration layer, then another class which makes calls to that database class? Thanks for your help

Posted: Wed Apr 13, 2005 12:06 am
by infolock
well, i kinda agree, but at the same time i think it would benefit to have it all in the same class. This is mainly due to the fact of the global variables that you could pass between each function.

For instance, say you are querying a database for some informatoin (say a field called name). the whole point of querying the database for this name is because you are going to change it to something else (for example : say a user is on a forum, and decies he/she wants to change his name. better yet, say it's a product description that you are gonna change to be a different name now).

Now, let's say that after you fetch the information, you are gonna need to either insert it into another table/field, update it, or whatever.

You are only going to be able to extend one class. You could, theoretically, create a daisy chain of links, but it would be a lot easier if you could say "$this->result" wherever you need to update.

example :

Code: Select all

class myclass
{
var $result, $SQL, $row;
  function myquery($mystring)
  {
   $this->SQL = mysql_query($mystring);
   $this->Fetch_Assoc($this->SQL);
  }
  function Fetch_Assoc()
  {
   $this->row = mysql_fetch_assoc($this->SQL);
   $this->result = $this->row['name'];
   $this->UpdateTable();
  }
  function UpdateTable()
  {
   $this->query = mysql_query("Insert into mytable (name) VALUES ('".$this->result."')");
  }
}
of course this is very cut and dry example. just wanting to show you how this could benefit you, and how it beats having to extend a class multiple times thought other classes just to have access to all the other mysql functions/variables you need to access...

of course, you can do however you want, and your scripting may not need this type of method, but it's how i built my class..

and no, i don't mean it looks like the above, but some functions follow the guidelines of passing global variables around when/if they are gonna need resulting data later on.

others can have other opinions about the matter, but this method works for me. it may or may not work for you, but that's the joy of coding ;) one method doesn't necessarily denote the best method.

Posted: Wed Apr 13, 2005 6:20 am
by timvw
i don't like classes that use global constants...

defeats the whole idea of encapsulation...

Code: Select all

function db() {
        $this->db = DB_DB;
        $this->user = DB_USER;
        $this->host = DB_HOST;
        $this->pass = DB_PASS;

Posted: Wed Apr 13, 2005 6:27 am
by SBro
So you think I should just do the following instead of using globals ?

Code: Select all

function db() {
    $this->db = 'database';
    $this->host = 'hostname';
    $this->user = 'username';
    $this->pass = 'password';
? thanks.

Posted: Wed Apr 13, 2005 6:30 am
by CoderGoblin
I would also separate query from Insert, Update and Delete, with the latter 2 setting a flag affected_rows which may also be retrieved with a method, or possible as the return value. When inserting a MySQL row I believe you can get the "id" inserted if autogenerated, Other databases (Postgres being the one I am familiar with) may not be able to. If all you ever use is MySQL then you may want to have an insert method returning this value.

Posted: Wed Apr 13, 2005 6:34 am
by timvw
SBro wrote:So you think I should just do the following instead of using globals ?

Code: Select all

function db() {
    $this-&gt;db = 'database';
    $this-&gt;host = 'hostname';
    $this-&gt;user = 'username';
    $this-&gt;pass = 'password';
? thanks.
depends. i would make a constructor that accepts db, host, uid and pwd as parameters...


and create my objects like $db = new db(DB_HOST, DB_USER, ...);

or i would make a factory that knows all the DSN (datasource names = host, user, ...) and then use that to generate an object

$db = $factory->getDB('name');

(only factory needs to know the link between name and settings...)

Posted: Wed Apr 13, 2005 9:08 am
by McGruff
Some more ideas in this article.

Posted: Fri Jun 03, 2005 1:44 pm
by xfish5
It is not bad class, but constants.. :?

Code: Select all

$db = new CMysql($host,$login,$pass,$db);
Simple and nice :)