Extendable PDO Wrapper Class

I've created a PDO wrapper. I understand PDO could be used on it's own, and I'm reinventing the wheel as there are solutions like Laravel's Eloquent that would do a better job.

I'd like advice on best practices used here, which would be much appreciated. The idea is to have a central Database class and separate classes for each implementation (e.g. MySQL, SQLite).

Database.php

pdo = new \PDO("mysql:host={$credentials['host']};dbname={$credentials['database']}", $credentials['user'], $credentials['password']);

                    // Turn on error output from PDO if debugging is on
                    if($this->debug) {
                        $this->pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
                    }
                break;
            }
        }
        catch(PDOException $e) {
            die(($this->debug) ? $e->getMessage() : '');
        }
    }

}

DatabaseInterface.php

<?php

interface DatabaseInterface
{
    public function table($table);
    public function insert($data);
    public function exists($data);
    public function query($sql);
    public function where($field, $operator, $value);
    public function count();
    public function get();
    public function first();
}

MySQLDatabase.php

``
'localhost',
'user' => 'root',
'password' => 'root',
'database' => 'gandalf'
]);
}

/**
* Chooses a table and returns self
*
* @param string $table
*/
public function table($table)
{
$this->table = $table;

return $this;
}

/**
* Inserts an array of data into the corresponding fields
*
* @param array $data
*/
public function insert($data)
{
$keys = array_keys($data);

$fields = '' . implode(', ', $keys) . '`';
$placeholders = ':' . implode(', :', $keys);

$this->stmt = $this->pdo->prepare("INSERT INTO {$this->table} ({$fields}) VALUES ({$place

Standard disclaimer:

I realize that some of the things I write here can come across as offensive. You should know that, IMHO, that's what code review has to be. It needs to be harsh, and blunt. Just keep in mind that I'm not trying to put you down. My only goal is to provide helpful, constructive, and above all honest feed-back. I've explained my views of what code-review has to be here.

I've covered PDO wrappers, and classes that extend it before. in detail. Yes, you say you realize you are reinventing the wheel, and that there are other, and better things out there. Great.

So why are you still persisting? Why not direct your attention to another project that actually stands to benefit from a consistent API?

I'm going to assume that you chose to write this PDO wrapper specifically because you have examples to draw from, and you seem to be somewhat familiar with its API. If these assumptions are incorrect, let me know.

For now, I'm going to focus on your approach/code, assuming that this is why you chose to write it.

I'm going to be updating this answer, because I've actually got quite a lot of work of my own to get done today, too, but no matter.

There are a couple of issues with your code that I consider to be offensive, hence I couldn't resist posting this.

The good parts

Well, at first glance, you seem to be aware of the coding standards, and you seem to be sticking to them. That's great. Keep it up.

The database class

Right of the bat: the constructor is evil. It should not be allowed to exist. Ever. Get rid of it. Now.

Why? Because you are writing a wrapper. A wrapper is just a container for a particular object, that gives the user access to the functionality of the wrapped object in a controlled way. A wrapper is not allowed to control the flow of the application that uses it, nor should it be the only place where you can control the "mode" in which to run the app.

Your constructor, however violates these simple principles:

try {
    //inside a pointless switch
    if($this->debug) {
        $this->pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
    }
} catch(PDOException $e) {
    die(($this->debug) ? $e->getMessage() : '');//controlls the flow of the app.
}

What if I wanted to use your code? I'd write it in such a way that the user is responsible for handling errors and exceptions. I, who wrote the class, don't know if my code is key for the rest of the code to work.

In PHP, a DB is most likely crucial, but that's not an absolute. Expect the unexpected. The user of your code should be expected to write:

try
{
    $db = new Database($type, $credentials);
}
catch (PDOException $e)
{
    //handle exception, if only: present a formatted message for the client.
}

Something else you should consider, in the spirit of "I-don't-know-where-this-class-will-be-used" is to allow for the user to pass attributes. It's the user who knows best what type of DB he's connecting to (what the default charset is, how he wants a NULL result to be returned etc...). So naturally, it falls to the user to set the specifics of the connection used.

It should, therefore be possible for the user to pass an instance of PDO, all set up for the job, to the constructor.

$credentials is expected to be an associative array. I know this because I looked at the function body of your constructor. I shouldn't have to. I should be able to see that by looking ad documentation, and the methods' signature:

/**
 * Expects $dbType to be mysql, sqlite, mssql, uri, ...
 *      defaults to MySQL
 * $credentials is an assoc array, keys host, database, user and password
 * @param string|PDO $dbType
 * @param array $credentials
 */
public function __construct($dbType, array $credentials)
{
}

As far as the DB type is conserned: add class constants. You know which DB types are supported, and which are not.

That's it for now, but there's a lot left to be said. A small teaser of what's to come:

public function insert($data)

Even though you are using prepared statements: you are vulnerable to injection! You are using the array $data that is being passed without any form of sanitation or validation whatsoever, to construct a string which will eventually generate your prepared statement. What if I pass something that isn't an array? What if I pass a numerically indexed array? What if I pass an array like this:

$db->insert(array(
    '; SELECT * FROM users; --' => null
));

Always expect the data to be malicious.

First update

Something I forgot to mention earlier, but a good place to start this update: As it stands, it is possible to create instances of your Database class. It shouldn't be, though. The class consists of a constructor, that assigns a protected property and nothing else. If that's the case code like this:

$db = new Database('mysql', $credentials);

wouldn't show up in any error log, nor will it cause exceptions to be raised, but what I end

StackExchange Code Review Q#58911, answer score: 4