[PHP Coders] Criticize this! Lets compare!

[HotelUser]

Hey there!

It's not so much that I'm unhappy with this SQL class, or that it's dysfunctional (although if you have an improvement for it I'd love to hear about it), I'd just like to compare and contrast with with other user's coding techniques. I suppose I could have posted this on Stack Overflow but I would really like to see how other's code here who I could probably relate to more:

Show Hide

$g_count = 0;
class mysqlDb
{
    public $con;
    public $debug;
    public $count;

    function __construct($host,$username,$password,$database)
    {
                if($GLOBALS['g_count'] == 0)
                {
                    $this->con = mysql_connect($host,$username,$password);
                    mysql_select_db($database, $this->con);
                }
                $GLOBALS['g_count']++;
    }

    function kill()
    {
        $GLOBALS['g_count']--;
            if($GLOBALS['g_count'] == 0)
        mysql_close($this->con);
    }

    function debugOn()
    {
    $this->debug = true;
    }

    function debugOff()
    {
        $this->debug = false;
    }

    function select($query,&$array)
    {
        $c = 0;
        $result = mysql_query("SELECT ".$query);
        $this->count = mysql_num_rows($result);
        if($this->debug == true)
             echo "SELECT ".$query;
        while($row = mysql_fetch_array($result))
          {
            foreach($row as $id => $value)
            {
                $array[$c][$id] = $value;

            }
            $c++;
          }
    }

    function update($update, $where,$array, $math = NULL)
    {
        if(!is_array($math))
        {
            foreach($array as $id => $value)
            {
                mysql_query("UPDATE {$update} SET `{$id}` = '{$value}' WHERE {$where}");
                if($this->debug == true)
                     echo "UPDATE {$update} SET `{$id}` = '{$value}' WHERE {$where}";
            }
        }else{
            foreach($array as $id => $value)
            {
                if(isset($math[$id]))
                    $statement = "UPDATE {$update} SET {$id} = {$id} {$math[$id]} {$value} WHERE {$where}";
                else
                    $statement = "UPDATE {$update} SET {$id} = '{$value}' WHERE {$where}";
                mysql_query($statement);
                if($this->debug == true)
                     echo $statement;
            }
        }
    }

    function delete($t, $w)
    {
        mysql_query("DELETE FROM `{$t}` WHERE {$w}");
        if($this->debug == true)
                 echo "DELETE FROM `{$t}` WHERE {$w}<br><br>";
    }

    function insert($where, $array)
    {
        $sql = "INSERT INTO `{$where}` (";
        $sql2 = " VALUES (";
        foreach($array as $id => $value){
            $sql .= "`{$id}`, ";
            $sql2 .= "'{$value}', ";
        }
        mysql_query(str_replace(', )',')',$sql.")") . str_replace(', )',')',$sql2.");"));
        if($this->debug == true)
             echo str_replace(', )',')',$sql.")") . str_replace(', )',')',$sql2.");")."<br><br>";
    }

    static function clean(&$x)
    {
        $x = mysql_real_escape_string($x);
    }

    static function cleanArray(&$x)
    {
        foreach($x as $id => $v)
        $v = mysql_real_escape_string($v);
    }
} 

If anyone else here has a similar setup, or, would like to code their interpretation of how they would code a class to complete a similar task, please do!

[LMS16]

Do you know what, I dont even go that far into PHP. I stick to basic that does the same job and is just a secure.

The only time Id go that far into it is when I work for a company lol

Lew.

[MattFr]

Looks good, you've used " where you would be better off using ' in a few places though! Wouldn't a singleton design pattern be best for a database class, or is PHP to useless for that?

[BoyBetterKnow]

I don't think I would personally have the two static functions as part of that class but I never do mysql classes solely because I only use mysql databases as a standard on my db class :\

I like the code. Only think I don't link is some of the names you've given variables and obviously that's personal pref

[Irrorate]

Did you consider using MySQLi or PDO? Half of them functions wouldn't be necessary if you did

[Dentafrice]

Here's a few things I would have done differently... I'm not sure what the g_count is... other then a connection count? Seems a bit pointless to a whole new instance of the class just for each database connection... but that's just me.

I usually keep all mine referenced inside of one database class, and do something like this when setting it...

<?php
include "classes/database.class.php";

$database_engine = new Database();

// lets create a new connection //

$database_connection = $database_engine->create("localhost", "blah username", "password", "database");

// lets do a query... //

$query = $database_engine("SELECT * FROM `users` WHERE `username`='Caleb'", $database_connection);
?>

Another thing... the debug on, debug off... could be simplified like this...

function debug($status = true)
{
    if ($status) {
        $this->debug = true;
    } else {
        $this->debug = false;
    }
} 

Saves from having two functions... and can just pass it a bool and you're off to the races.

Other then that, looks pretty good...

[HotelUser]

Here's a few things I would have done differently... I'm not sure what the g_count is... other then a connection count? Seems a bit pointless to a whole new instance of the class just for each database connection... but that's just me.

I usually keep all mine referenced inside of one database class, and do something like this when setting it...

<?php
include "classes/database.class.php";

$database_engine = new Database();

// lets create a new connection //

$database_connection = $database_engine->create("localhost", "blah username", "password", "database");

// lets do a query... //

$query = $database_engine("SELECT * FROM `users` WHERE `username`='Caleb'", $database_connection);
?>

Another thing... the debug on, debug off... could be simplified like this...

function debug($status = true)
{
    if ($status) {
        $this->debug = true;
    } else {
        $this->debug = false;
    }
} 

Saves from having two functions... and can just pass it a bool and you're off to the races.

Other then that, looks pretty good...

I think you're right about the debug function, I should put it into one. I had to make g_count because if I had multiple instances of an sql object open at once before I called kill() it would error me because I was trying to open two connections to the database at once. I guess I could have connected to the database at the top of every script and then not had to worry about this but I suppose I wanted my functions to work more standalone which is why I wrote this sort of messy trick in

[Dentafrice]

Hmm, gotcha. I just like to keep everything of mine inside the class, without relying on anything outside of it

[HotelUser]

Fair enough. Global variables can get very hairy when they're abused

[MattFr]

I think you're right about the debug function, I should put it into one. I had to make g_count because if I had multiple instances of an sql object open at once before I called kill() it would error me because I was trying to open two connections to the database at once. I guess I could have connected to the database at the top of every script and then not had to worry about this but I suppose I wanted my functions to work more standalone which is why I wrote this sort of messy trick in

Use Singleton.