Extendable PDO Wrapper Class

Question

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

<?php

class Database
{
    /**
     * The PDO instance
     *
     * @var PDO
     */
    protected $pdo;

    /**
     * If database debugging should be turned on
     *
     * @var bool
     */
    public $debug = false;

    public function __construct($type, $credentials)
    {
        try {
            switch($type) {
                case 'mysql':
                default:
                    $this->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

<?php

class MySQLDatabase extends Database implements DatabaseInterface
{
    /**
     * A stored statement
     *
     */
    protected $stmt;

    /**
     * The selected table
     *
     * @var string
     */
    protected $table;

    public function __construct()
    {
        parent::__construct('mysql', [
            'host' => '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 ({$placeholders})");

        return $this->stmt->execute($data);
    }

    /**
     * Check if a record exists with the specified parameters
     *
     * @param array $data
     */
    public function exists($data)
    {
        $field = array_keys($data)[0];

        return $this->where($field, '=', $data[$field])->count() ? true : false;
    }

    /**
     * Query and return the result
     *
     * @param string $sql
     */
    public function query($sql)
    {
        return $this->pdo->query($sql);
    }

    /**
     * Get a records where the given field matches the given value
     *
     * @param string $sql
     * @return Gandalf\Database
     */
    public function where($field, $operator, $value)
    {
        $this->stmt = $this->pdo->prepare("SELECT * FROM {$this->table} WHERE {$field} {$operator} ?");

        $this->stmt->execute([$value]);

        return $this;
    }

    /**
     * The count of the last executed query
     *
     * @return int
     */
    public function count()
    {
        return $this->stmt->rowCount();
    }

    /**
     * The results for the last executed query
     *
     * @return object
     */
    public function get()
    {
        return $this->stmt->fetchAll(\PDO::FETCH_OBJ);
    }

    /**
     * The first result for the last executed query
     *
     * @return object
     */
    public function first()
    {
        return $this->get()[0];
    }
}

Answer 1

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 up with is utterly, and completely pointless. I can't do anything with the DB connection, because its properties are not accessible to me. Make the class an abstract, or add some basic methods to it that make the class, well... usable.

Another good thing

I also forgot to mention a good part earlier: You are using an interface. That's great. Truly great. But judging by the code you posted, I'm worried you might be missing out on some of the goodies that interfaces offer (more on that in the critiques below).

Critiques

You might be missing out on some of the interface goodies. Ok, what do I mean by that. Well, I can't help but noticing you're not using type-hints anywhere in your code. You really should. I promise: write one project using type-hints, and you'll soon find yourself wondering how you could've ever done without.
Type-hints are nothing more, or less, than things that tell PHP that a given function/method should not be allowed to work if its argument(s) aren't of a particular type. This facilitates debugging, and makes your code more self-documenting.

For example: I have a function called "setAttributes", and I write it so that it sets attributes on a certain type of objects (using getters and setters). In the olden days, I'd have written:

function setAttributes($object)
{
    if (!is_object($object))
        throw new InvalidArgumentException(
            sprintf(
                '%s expects argument to be an object (of classes X or Y)'
                __FUNCTION__
            )
        );
    if (!$object instanceof X && !$object instanceof Y )
        throw new InvalidArgumentException(
            sprintf(
                '%s expects argument to be instanceof X or Y, not %s'
                __FUNCTION__,
                get_class($object)
            )
        );
    //the actual code
}

That's just an awful lot of code, just to make sure the type of one argument matches the expectated type. Type-hints that are not respected cause the code to fail immediately:

function test(array $foo)
{
    return 'This is only returned if $foo was an array';
}
test(array(123));//returns string
test(123);//fails

That's great, but remember the setAttributes function's job: to use setters and getters on a number of classes. Well, I can make sure these methods exist, by having these classes implement an interface, and use a type-hint to require the argument to be a class that implements that very same interface:

class X implements Attributes{}
class Y implements Attributes{}
function setAttributes(Attributes $inst)
{
    $inst->setAdvantage('No checks required here, this method HAS to exist');
}

What's more: any half-decent IDE will tell you when you're passing an incompatible variable to this function, and if you document your code well, you'll see just how much easier coding gets when you have autocompletion :).

The big one

At first glance, your MyDatabase class looked pretty clean, and concise. After taking a closer look, I'm afraid I must say all is not well.
I've already mentioned that your Database constructor was flawed, for not letting the user decide how to connect to the database. Given that the Database class is pointless to instantiate, the user has no choice but to use classes like MyDatabase. The example you posted here makes the Database::__construct problem even worse: it takes no arguments, and passes a hard-coded array to its parent constructor. This is wrong on just about all levels:

• It makes your code even harder to test
• It makes it highly unlikely you're writing portable code
• any instance is even less configurable than it was
• Worst of all: it violates the contract, imposed by the parent details on DbC

Explanation:

How come? Well, if you can't pass the connection parameters to the constructor, you can't connect to a test DB/mock. At least not as easily as otherwise you might have done.
It's unlikely your code is portable, because the chances of another, separate project sharing the same DB structure are very slim indeed.
You can't pass anything to the constructor, hence you can't configure anything. In your parent (Database) class, you could at least specify which database, what user and which adapter to use.

But the biggest issue, by miles is the breach of contract. That is just unforgivable. It defies common sense, and will bite you in the end.
Consider this:

class Foo
{
    public function someMethod(array $argument)
    {
        //do stuff
    }
}
class Bar extends Foo
{
    public function someMethod(stdClass $arg)
    {
        //other stuff
    }
}
//somewhere else in your code:
function useInstance(Foo $inst)
{
    $inst->someMethod(array(123));
}

As far as the useInstance function is concerned, things are pretty straightforward: it expects to be passed an instance of Foo, so it can safely assume the argument has a method called someMethod. If it needs a Foo, it makes perfect sense for the function to assume the method's signature is Foo::someMethod(array $argument);. But what if I pass an instance of Bar? Inheritance means that every instance of Bar is an instance of Foo, that is extended (added functionality). Changing the signatures of overridden methods is NOT DONE, simply because it breaks compatibility between the child and the parent. It breaks the contract, and defeats the whole point of OOP.

It took me a while to truly grasp this concept, too, but here's a clear exposé on the matter

Other niggles

public function table($table) is about as vague a method as it gets. What does it do? Is it a getter, a setter? If it's a getter, does it return a string, or an instance? No way of knowing, other than looking at the code itself. In which case, why would I want to use this wrapper, if I have to look everything up?
This method also shares its name with the property $table, which makes your code more error-prone, a typo like $this->table(); when you meant to write $this->table; (property access) won't show up as readily in your IDE.
Do what all sane people do, and call the method like it is: it's a setter. Use it accordingly: never set anything without sanitizing/validating

/**
 * @param string $tableName
 * @return $this
 */
public function setTable($tableName)
{
    $name = trim($tableName);
    //example, not actual validation
    if (preg_match('/[%@$#\.+\-=(){}"\';\s\t[]]/', $name))
        throw new InvalidArgumentException(
            sprintf(
                '%s is not a valid table name: contains illegal chars'
                $name
            )
        );
    $this->table = $tableName;
    return $this;
}

---

Some general, smaller issues:

So far, I've covered the __construc, table and insert methods. Now I'll touch on the rest of the MyDatabase class' methods.

exists:
This method merely acts as an alias for where, only: I can pass any number of fields + value combinations to it I want, only the first value will be used. That's far from ideal, is it?
I also have no way to use wild-cards. The operator you use will always be =, never LIKE. Now I'm not a fan of LIKE, but occasionally, I find myself using it.

query:
I couldn't help notice that, throughout your code, you same to make sure the PDOStatement returned by the PDO calls is not exposed to the user, yet this method blindly passes on a query to PDO (without preparing the statement, without validation, without any form of security checks) and returns the PDOStatement instance.

where:
I'd expect this method to be present on an abstraction layer that actually allows me to build a query. Put yourself in the shoes of someone who first sees your code being used, and think for a second what he might think that happens, and compare that to what actually happens:

$db = new MyDatabase();
$db->where('foo', '=', 'bar');
$head = $db->first();

That code might make sense to you, but to me, it looks as if the $db instance is both a query, and a dataset, and a database connection all at once. That's confusing to say the least.

count:
That's just a plain unsafe method: you don't even check if $this->stmt is an instance of PDOStatement. It might not be, so calling this method might cause a fatal error. In plain English: your code is not stable. The same goes for get and thus, first, too.

get:
A fetchAll call on an (unchecked) PDOStatement is not uncommon. But there is no limit on how many times this code might be called. Why, then, aren't you caching (as in assigning the data to a property)?
Consider the following scenario:

$first = $db->first();
$rest = $db->get();
array_shift($rest);

In this case, get will be called twice. Doesn't that look pretty inefficient and clumsy to you? Because it sure does to me.

One more thing:
There is one more issue to address: PDOStatement instances should be reusable, and users should have access to them. You are hiding the brilliant things PDOStatement instances can do for you.

---

Conclusion

I could rave on and on for quite some time, still, but this review has already gotten quite lengthy. I recommend you read through all the links I provided, and I urge you to take note of my critiques. You are of course free to think, and do what you want.

I guess it all boils down to this: what you're doing is taking a good tool, and ruining it. I wouldn't use your code, ever: I can't use PDOStatement::beginTransaction, I can't reuse the PDOStatement's at all. I can't set the connection attributes as I see fit, and can't inject my own PDO connection. Using your code would feel like voluntarily giving up all creature comforts and going back to live as a cave man. It's much like switching back from PDO to the old and deprecated mysql_* extension.