PDA

View Full Version : Database class for PHP 5+ and MySQL (Help)



Karlos
07-23-2009, 11:49 AM
Well I a total noice when it comes to OOP, so I was wondering if anyone could give me any tips, idea's or constructive critism.

<?php
if (!defined('Access')) { die('Forbidden Access.'); }

Class Database {
protected $Connect, $ConnectStatus = false;
public $TotalQueries, $Affected = null;

public function __construct($Host, $User, $Pass, $Database) {
$this->Connect = @mysql_connect($Host, $User, $Pass);
if (!is_resource($this->Connect)) {
$this->ConnectStatus = false;
die('Cannot Connect To The MySQL Server.');
} else if (!@mysql_select_db($Database, $this->Connect)) {
mysql_close($this->Connect);
$this->ConnectStatus = false;
die('Cannot Select The Defined MySQL Database.');
} else {
$this->ConnectStatus = true;
}
}

public function isConnected() {
return $this->ConnectStatus;
}

public function doEscape($Var) {
if (get_magic_quotes_gpc()) {
$Var = stripslashes($Var);
}
return mysql_real_escape_string($Var, $this->Connect);
}

public function doQuery($Sql) {
$Query = @mysql_query($Sql, $this->Connect);
if ($Query === false) {
return $this->raiseError();
}
++$this->TotalQueries;
$this->Affected = @mysql_affected_rows($this->Connect);
return $Query;
}

public function affectedRows() {
return mysql_affected_rows($this->Connect);
}

public function numRows($Sql){
return mysql_num_rows($Sql);
}

public function insertId() {
return mysql_insert_id($this->Connect);
}

public function doDisconnect() {
$this->ConnectStatus = false;
return mysql_close($this->Connect);
}

}

?>

Haunted Dawg
07-23-2009, 04:20 PM
What you could do with your..



public function doEscape($Var) {
if (get_magic_quotes_gpc()) {
$Var = stripslashes($Var);
}
return mysql_real_escape_string($Var, $this->Connect);
}


.. function is build onto it and make it more reliable such as..



public function doEscape($Var, $Type = 0) {
return ($Type == 1) ? abs(intval($Var)) : mysql_real_escape_string($Var, $this->Connect);
}


For cleaning numbers:



$db = new Database('localhost','username','password','databa se');
$db->doEscape($input, TRUE);


for cleaning texts..


$db = new Database('localhost','username','password','databa se');
$db->doEscape($input);


or



$db = new Database('localhost','username','password','databa se');
$db->doEscape($input, FALSE);

Karlos
07-23-2009, 09:20 PM
How ever, that is a valid point, but if you filter the data which in inputed, before entering it into the database, I would personally make sure it's a whole number :-)

Zeggy
07-24-2009, 01:04 AM
Put doDisconnect into the __destruct function.

Karlos
07-24-2009, 08:52 AM
public function __destruct() {
$this->ConnectStatus = false;
return mysql_close($this->Connect);
}

If I got it right? Should be, never used it though :-P

Floydian
07-26-2009, 11:37 AM
Yeah that's it Karlos.
My personal opinion is that the close function should be it's own method.

Database::close();

And then the destructor should call Database::close();
A minor detail, and it's my personal pref. What you have is theoretically sufficient since it's a good assumption that the connection would only be closed upon destroying the DB object, but it's not quite as thorough.

-------------------------

I noticed you've got your property declarations stacked:

protected $Connect, $ConnectStatus = false;
public $TotalQueries, $Affected = null;

That's fine, and I used to do that as well (the Horizons Database class has it, in fact :P ), but I've since learned that in order to get code documentation and code completion in IDE's, those really need to be seperate lines.



/**
* The MySQL connection resource
*
* @var resource
*/
protected $Connect;

/**
* Are we currently connect?
*
* @var boolean
*/
protected $ConnectStatus = false;

/**
* How many queries have we done?
*
* @var integer
*/
public $TotalQueries = 0;

/**
* How many rows were affected by the last query.
*
* @var mixed
*/
public $Affected = null;


Now, when you type in $this->Conne ---- you will see the doc block above Database::Connect in a pop up to remind you or others using your code of what Database::Connect is for.


Depending on the IDE and how it handles this stuff, you can get code documentation for public references to the class as well.

$db = new Database();

$db->Con ---- and you will get code documentation here as well.

If you do something weird like:

$db = Database::getInstance();

This would be the *singleton* instantiation method, you're getInstance doc block should have a:


@return Database

Which will tell the IDE that $db = Database::getInstance(); is creating and storing a Database object in $db.

Vali
07-31-2009, 04:10 AM
Your class is useless... it's just a wrapper for the normal PHP functions, and to use it, it would be harder than to use normal PHP function calls...

Here are a few suggestions:
- try not to use @, that's the slowest thing in PHP
- instead of "die", throw an exception (do you really want to show that warning to the user for everyone using your class?)
- split up your SELECT, UPDATE, INSERT, DELETE in different functions, that way, you can do more things depending on what query the user does (ex: count the number of selects vs updates)
- use '__deconstruct", if you really want to kill the connection once your done (or store the class stats in the session, so the next refresh can use them)
- you could probably make every function in there STATIC, depending on how you plan to use your class (ex: if you always use it with the same DB)
- etc...

hope that helps.

Floydian
07-31-2009, 05:48 PM
you could probably make every function in there STATIC

Doing that means that all the properties need to be public, and that means that you have no visibility control.
Being able to use $this gives you visibility control.

If you want to ensure that only one instance of a class is instantiated, the singleton pattern should be used instead of making everything in the class static.

The standard php singleton pattern uses a getInstance() method, which gives access to the singleton in any scope, so you get the best of static class methods, but also the visibility control of non static classes.

Hope that helps.

Vali
08-01-2009, 03:36 AM
Depends on what you need. And if you don't need $this, you should use STATIC methods, since they are about 4 times faster than normal ones.

Floydian
08-01-2009, 04:01 PM
Vali, you didn't reply at all to what I said about visibility...

Vali
08-01-2009, 09:04 PM
There is nothing private or protected that you need for a DB class...

Floydian
08-02-2009, 04:32 AM
I'd definitely disagree with that. Just for a basic refutation of your argument Vali, let's consider a query counter.

You can have a static counter, but then anyone, anywhere, can set it anytime. I.e., the principle of encapsulation is broken. A query counter should only be set internally by the database class.


You may say this is trivial, but if the query counter can be blown out, then the db connection resource can be blown out! Without setting the connection resource as protected/private, you cannot be guaranteed that your connection won't be blown out.

Now, I respect that you personally don't see the value in this, so I'm only posting this response for the folks that may need to know this information.

Vali
08-02-2009, 05:01 PM
Here you go:



<?php
/**
* This is your DTO that holds your counters (can be used by more than one class)
*/
class myCounterDTO {
public static $count = 0;
}

/**
* This class does stuff
*/
class myClass {
public static function doStuff() {
myCounterDTO::$count ++;
echo "Doing stuff!\n";
}
}

/**
* This class does other stuff
*/
class myOtherClass {
public static function doOtherStuff() {
myCounterDTO::$count ++;
echo "Doing stuff!\n";
}
}

// Do some stuff and other stuff
myClass::doStuff ();
myOtherClass::doOtherStuff ();
myOtherClass::doOtherStuff ();

// See how many times we did stuff
echo "Did stuff: " . myCounterDTO::$count . " times.\n";
?>


Usually, 'myCounterDTO' should be a friend class of 'myClass' and 'myOtherClass', but PHP doesn't have that...

The idea is that you can have multiple classes that increase your 'query' count.

Floydian
08-02-2009, 06:05 PM
I'm sorry to say, but your "dto" is also lacking encapsulation.

Firstly, it should have a myCounterDTO::increment() method so that the counter variable doesn't have to be modified directly. Secondly, since this is a static class, with all members static, it should be using the singleton pattern. If you did that, you could hide the counter variable, and only allow access to the methods that operate on it.

increment()
and
get()
perhaps a
reset()

Cheers

Vali
08-03-2009, 02:27 AM
The DTO class is just an object, that can be passed around. So you always know what your getting / setting.

As for the singleton, in this case, making it all static is WAY less code / faster to do, giving the same result.

Also, if all you have is one variable, it's just plain retarded to add 'increment()', 'get()' or 'reset()'...
I mean what's the point?

If you REALLY want to go out your way and waste your time, then I guess you can do it like that. It's only ~50 times slower and 20 times the code.

Floydian
08-03-2009, 04:27 AM
The DTO class is just an object, that can be passed around. So you always know what your getting / setting.


You don't know what you're getting if the value is public. Hence the need for visibility control, i.e., "So you always know what your getting / setting".

Cheers



It's only ~50 times slower and 20 times the code.


Since your DTO is no different than a plain old variable (i.e, no methods, and no visibility), I think you're "optimized" argument can actually be applied against your DTO. You might as well just use a variable and be done with it.

Vali
08-03-2009, 03:09 PM
If the variable is public, your getting that variable back... or what you put in it.

If your putting a blob in your counter, it's your fault for messing it up. There's no need to recheck all your variables all the time, just in case the programmer decided to do: $counter = 'five'; instead of $counter = 5;

Toppy
10-09-2009, 06:20 PM
My suggestion is as follows:

First, welcome to the world of PHP 5 and OOP.
Second, be careful and be prepared. When you ask a question about OO methodology it typically ends up in a huge debate about theory and how it "should" be done. I've seen more hours spent on debating OO than actual programming time on various apps. Its led me to wonder if dev's add that on to their bill or not?

PHP 5 OO Coders Bill:
Item: PHP Coding: 5 hours
Item: OO Practice Debate and Theories: 25 hours

I've also seen guys debating OO theory and when a simple PHP language construct question came up they had no clue about it!

Ok, thats my 2 cents. Good luck on your project and trying to figure out what is the best practice. :)

Floydian
10-10-2009, 12:28 AM
My suggestion is as follows:

[SNIPPED]be careful and be prepared [w]hen you ask a question about OO methodology it typically ends up in a huge debate about theory and how it "should" be done.


abstract class ForumPost {
protected $desc = 'be careful and prepared when you ask a question about many topics, it may end up in a huge debate.';

public function getDesc() {
return $this->desc;
}

abstract public function doPost();
}