PHP MySQL Database class

This is a PHP database class. Yes, I know it's using the MySQL functions, which are deprecated, but I shall be updating it to MySQLi soon. Can you please review this code and give any comment on any improvements or changes you think I should make?

```
Initiate();
}

final public function Initiate()
{
global $_CONFIG;
if(!$this->initiated)
{
$this->setMySQL('connect', mysql_connect);
$this->setMySQL('pconnect', mysql_pconnect);
$this->setMysql('select_db', mysql_select_db);
$this->setMySQL('query', mysql_query);
$this->setMySQL('num_rows', mysql_num_rows);
$this->setMySQL('fetch_assoc', mysql_fetch_assoc);
$this->setMySQL('fetch_array',mysql_fetch_array);
$this->setMySQL('result', mysql_result);
$this->setMySQL('free_result', mysql_free_result);
$this->setMySQL('escape_string', mysql_real_escape_string);

$this->initiated = true;

$this->connect($_CONFIG['mysql']['connection_type']);
}
}

final public function setMySQL($key, $value)
{
$this->mysql[$key] = $value;
}

/-------------------------------Manage Connection-------------------------------------/

final public function connect($type)
{
global $core, $_CONFIG;
if(!$this->connected)
{
$this->connection = $this->mysql$type;

if($this->connection)
{
$mydatabase = $this->mysql'select_db';

if($mydatabase)
{
$this->connected = true;
}
else
{
$core->systemError('MySQL Engine', 'MySQL could not connect to database');
}
}
else
{
$core->systemError('MySQL Engine', 'MySQL could not conne

namespace Revolution;
class engine

I do not understand these names. I would have expected to see something database related. Neither Revolution nor engine says database to me.

final public function __construct() 
{ 
    $this->Initiate(); 
}

final public function Initiate()
{
    global $_CONFIG;
    if(!$this->initiated)
    {
        $this->setMySQL('connect', mysql_connect);
        $this->setMySQL('pconnect', mysql_pconnect);
        $this->setMysql('select_db', mysql_select_db);
        $this->setMySQL('query', mysql_query);
        $this->setMySQL('num_rows', mysql_num_rows);
        $this->setMySQL('fetch_assoc', mysql_fetch_assoc);
        $this->setMySQL('fetch_array',mysql_fetch_array);   
        $this->setMySQL('result', mysql_result);
        $this->setMySQL('free_result', mysql_free_result);
        $this->setMySQL('escape_string', mysql_real_escape_string);

        $this->initiated = true;

        $this->connect($_CONFIG['mysql']['connection_type']);
    } 
}

As a general rule, you don't want to do your database connection in your constructor. You should be able to create an instance without having it connected to a database. What you might consider doing is creating a fetchConnectedInstance that runs the constructor and initializes it, returning the result. You should pass $_CONFIG into this function and Instance. That way you can connect to more than one database at once. Your version is almost a Singleton but not quite.

I don't see the point in things like

$this->setMysql('select_db', mysql_select_db);
        $mydatabase = $this->mysql['select_db']($_CONFIG['mysql']['database'], $this->connection);

Note that $this->mysql['select_db'] is longer than mysql_select_db, so it's clear that this is not a convenience abbreviation. What is the benefit? It almost seems like you are trying to make a base class that you can reuse for other database types. Unfortunately, I don't know that you can rely on each database offering the same abilities. For example, a key/value database is not going to offer some of the capabilities that a relational database will.

If you are going to alias something, then there should be a reason. If there's a reason, you should comment on it so that the reader knows it. As a general rule, if you are clever, explain to the reader how it's clever. That way, the reader doesn't need to be clever just to read the code.

I don't like the use of $this->initiated. It doesn't seem necessary. Either don't care about that situation, or check something more direct like the existence of $this->mysql.

/*-------------------------------Secure MySQL variables-------------------------------------*/

final public function secure($var)
{
    return $this->mysql['escape_string'](stripslashes(htmlspecialchars($var)));
}

This is not the right way to handle this. The htmlspecialchars function should not be called before putting something in the database. It should be called after fetching something from the database and before outputting it in HTML. It should be the last thing called before outputting. Doing it this way risks someone finding a special input that will evade htmlspecialchars and then be enabled by the stripslashes/mysql_real_escape_string. Also, htmlspecialchars can change the data if used at this point. What if you want to store HTML in the database? E.g. user entered content.

You also aren't using bind parameters at all. In general, bind parameters are considered a safer way of handling things than using mysql_real_escape_string.

if($this->mysql['close'])

What this does is check if $this->mysql['close'] has a true or false evaluation. You may have meant to do at least

if ($this->mysql['close']())

However, that's still incorrect. That closes the last database connection that was open. That may or may not be the right connection. It really should be

if ($this->mysql['close']($this->connection))

In the following two function signatures, you use $sql to represent two different things:

final public function query($sql)
final public function num_rows($sql)

In the first, $sql is a string representing a MySQL query (i.e. a SQL string). In the second, it is a query result handle. I find that $sql makes a lot of sense in the first signature and no sense in the second. I would suggest changing the second to something like

final public function num_rows($query)

You also have a function that doesn't quite do what one would expect:

final public function fetch_assoc($sql)
{
    return $this->mysql['fetch_assoc']($this->mysql['query']($sql, $this->connection));
}

What you do here is run a query based on the $sql string and then return the first row of the result. However, you now have no way to get subsequent rows, as you throw away the query result handle. You take care of this situation with a while i

StackExchange Code Review Q#67954, answer score: 3