Database class using PDO

Question

The point of this question

I'm actually using it while developing a simple application and it seems to cover all my needs. Also it uses PDO so that we don't really have to worry about SQL Injection. I know I usually code strange, but I hope you could give me suggestions and feedback in order to improve it.

Code: Database Class

/* Operate on the database using our super-safe PDO system */
class db
{
    /* PDO istance */
    private $db = NULL;
    /* Number of the errors occurred */
    private $errorNO = 0;

    /* Connect to the database, no db? no party */
    public function __construct()
    {
        try
        {
            $this->db = new PDO(
                'mysql:dbname='.reg::get('db-name').';host='.reg::get('db-host'), reg::get('db-username'), reg::get('db-password')
            );
        }
        catch (Exception $e) 
        {
            exit('App shoutdown');
        }
    }

    /* Have you seen any errors recently? */
    public function getErrors() { return ($this->errorNO > 0) ? $this->errorNO : false; }

    /* Perform a full-control query */
    public function smartQuery($array)
    {
        # Managing passed vars
        $sql = $array['sql'];
        $par = (isset($array['par'])) ? $array['par'] : array();
        $ret = (isset($array['ret'])) ? $array['ret'] : 'res';

        # Executing our query
        $obj = $this->db->prepare($sql);
        $result = $obj->execute($par);

            # Error occurred...
            if (!$result) { ++$this->errorNO; }

        # What do you want me to return?
        switch ($ret)
        {
            case 'obj':
            case 'object':
                return $obj;
            break;

            case 'ass':
            case 'assoc':
            case 'fetch-assoc':
                return $obj->fetch(PDO::FETCH_ASSOC);
            break;

            case 'all':
            case 'fetch-all':
                return $obj->fetchAll();
            break;

            case 'res':
            case 'result':
                return $result;
            break;

            default:
                return $result;
            break;
        }
    }

    /* Get PDO istance to use it outside this class */
    public function getPdo() { return $this->db; }

    /* Disconnect from the database */
    public function __destruct() { $this->db = NULL; }
}

Use

$db = new db;
$user = $db->smartQuery(array(
    'sql' => "SELECT UserName FROM `post` WHERE UserUID = :uid",
    'par' => array('uid' => $uid),
    'ret' => 'fetch-assoc'
));
echo $user['Username'];

What I think is wrong

Well, I have encountered these 2 points while revisiting this code, and I'd like to get some feedback about them particularly:

1. The error system (which, let's face it, sucks right now)
2. The first try-catch code, which is actually working, but I never use that so, please, look at it.

Also if my application cannot connect to the database, most (if not all) features cannot be activated (such-as the PHP error log trough a database record, so that every times an error occurred, the admin is warned trough the application itself).

Answer 1

A few things.

When you connect, don't hard code mysql, you are already fetching all the db info from a registry/config ... make the the dbtype configurable as well. That is after all, the point of PDO.

When you catch errors in connection or when executing a query, use the PDOException class

use exceptions for your prepare and execute calls instead of counting the errors that happens...as that's not really useful (it will be obvious an error happened, what I care about is what that error actually is).

personally, I would break down this class into specific methods. instead of connecting to the db in the constructor, I would have a connect() method to do this. I would also have a query, prepare, and bind method and then have interaction method: delete, update, insert and then a set of fetch methods, fetchOne, fetchAll, fetchCol, etc.

Also the connect method would check for a set conn, and only attempt to connect if you didn't already have an existing connection. The query method would call connect(). The reason for this is that you may include your db class throughout your code, even if you dont fetch anything in particular requests...with your code if this is the case a connection will be made regardless. What you want is lazy loading...meaning a db connection will only be made if you actually attempt to use the db with a fetch/update/delete/insert/etc.

Here is some sample pseudo code

$db = new MyPdoClass();
$db->connect();
$stmt = $db->query($sql, $params);
$results = $stmt->fetchAll(PDO::FETCH_ASSOC);
// or call your MyPdoClass::fetchAll, which would do that so you would just call
$results = $db->fetchAll($sql, $params);

Answer 2

Why not make DB a Singleton class?

Making it Singleton will prevent multiple sign-ons. Now in your code, each time a DB object is created, you authenticate with the database server.

If you make it Singleton, you just have to connect once. And whenever you need the instance, you just have to fetch it.

Answer 3

Coupling

The first thing that comes to mind is the tight coupling between db and reg.

Since db doesn't actually need to know anything about reg, and only needs a few values from it, I feel it's better practice to pass the required values in as needed:

public function __construct($host, $database, $username, $password)
{
    ...
}

Exception handling

As mentioned in another answer, catch PDOException instead. Also, you will have a lot of frustration down the track trying to figure out what an "App shoutdown" error means with improved error reporting.

try
{
    $db = new DB(...);
}
catch (PDOException $e)
{
    logError($e);
    exit('Sorry, the site is broken!');
}

Commenting

phpDoc comments can prove invaluable as the size and complexity of classes grow. Even if not strictly necessary for simple projects, I've found it a useful habit to get into.

Answer 4

I'd avoid Singletons - you can limit access by using a dependency injector like YADIF, which makes writing tests against components that rely on the connection easier to create (as well as testing the class itself)

This is a tricky question - DBAL (Database Abstraction Layers) is a deceptively short acronym for a very broad (and hotly debated) topic.

If you want to keep it light, you can write a Transaction script:

http://martinfowler.com/eaaCatalog/transactionScript.html

Basically an OOP wrapper for your apps most common queries (i.e. saveCompanyEmployees, deleteEmployee, selectEmployee etc.).

You can take this up a notch and use assemble queries programmatically, in which case your methods may become more like this:select($table,$id), selectWhere($table,$condition) - this is more of a Data Access Object than a Transaction Script

If you find yourself moving towards a fluent interface, or class representations of tables - then you may well want to consider an ORM of some description (it's not really worth writing your own, although it has the advantage of being more tailored to your app and possibly more lightweight, and of course you learn the hard way - which is often the best way...), although these can be a mixed blessing - and personally I think it's slightly perverse to be so averse to SQL to hide it entirely in this manner.

I'd reserve your direct SQL execution as a "back door" method you can resort to when all else fails - since having to write the SQL in your application logic kinda ruins the encapsulation benefits of OOP

Answer 5

I'm assuming you're using PHP5, in which case do you have a good reason not to use existing libraries like Doctrine2 DBAL the defacto standard for database abstraction, or Pear_MDB2?

In this day and age of open source libraries, rolling your own home made (and possible error prone class) to solve already thoroughly solved problems is doing yourself and the PHP community at large more harm than good don't you think?

These libraries are well documented (not so much in the case of Pear_MDB2), thoroughly tested and have 1000s of hours of production use in a myriad of environments. And shoudl you find an issue or have a suggestion you'd be helping the PHP community. At the very least have a look at these fantastic projects, you might learn something and possible even answer some of your own questions should you insist on doing your own way.

Admittedly Pear_MDB2 is a bit behind the times now, but it does a solid job