Is coding to an interface with regards to PDO the right thing to do?

Am I on the right lines for connecting to my Database? If not could somebody help me improve it or point me in the right direction?

Index.php:

use Acme\Foo\Database\Connector;
use Acme\Foo\Database\MySQLConnector;

$obj = new Connector(new MySQLConnector);
echo $obj->db->connect();

DatabaseInterface.php:

interface DatabaseInterface {
    public function connect();
}

MySQLConnector.php:

use Acme\Foo\Database\DatabaseInterface;
use PDO;

class MySQLConnector implements DatabaseInterface
{
    public function connect()
    {
        $user = 'root';
        $pass = 'root';
        try {
        return new PDO('mysql:host=localhost;dbname=test', $user, $pass);
        }
        catch (\PDOException $e)
        {
            throw new \Exception($e->getMessage());
        }
    }
}

Connector.php:

class Connector
{
    public function __construct(DatabaseInterface $db)
    {
        $this->db = $db;
        $this->db->connect();
    }
}

This feels quite over-engineered to me, when i can do the work of all three classes in one line of code.

echo new PDO('mysql:host=localhost;dbname=test', 'root', 'root');

The big question to ask here is: What do you gain?

Every class, every function, every line of code should be able to justify its existence. More code means more cognitive load -- that's more stuff to have to think about whenever you want to change/add/fix something, so you don't want to add stuff willy-nilly. It should either reduce overall complexity, add flexibility, or in some other way make the application better.

Here, you add two classes and an interface (adding complexity) and hard-code the connection params (reducing flexibility). So what makes these classes an improvement over the one line above? What do you gain from that indirection, that justifies the cost?

"Coding to an interface" doesn't necessarily mean using the interface keyword. "Interface" here is used in the language-agnostic sense, and basically refers to an API. When you code to an interface, you're simply using an API that doesn't make the consumer rely on implementation details. When you use PDO, to some extent, you are already doing that. The whole point of PDO is to hide a lot of the DBMS-specific stuff behind a common API, so all you have to know is the PDO API and general SQL stuff.

Since you're returning PDO objects, you're not really hiding much (if any) of the implementation. I still have to know PDO, on top of how to use your API to get a usable PDO object. You can't really use anything but PDO, because you've married your API to it. And if i want to use a different database, i have to create a new class.

A more useful abstraction would hide the PDO objects from the user as well, so that someone using this code only needs to know two things -- your API, and general SQL stuff. But at that point, you're starting to reinvent PDO.

As for the code itself, if you really want to keep it...

-
The name MySQLConnector suggests that it is a special type of Connector. It's not -- it's a special type of DatabaseInterface. Consider a different name. I might suggest TestDatabase if you're going to keep the connection info hard-coded, or MySQLDatabase if you make it general enough to be used for other databases.

-
Never throw new \Exception. There is never a good reason, other than laziness. And here, even laziness wouldn't suffice -- if you're not handling an exception, you can simply not bother to catch it. By catching it and throwing a whole new exception that's as generic as you can legally make an exception, you may be tossing out essential info -- like the previous exception's stack trace! -- that could save you days of debugging.

If you don't want the user to have to know that a PDO exception may occur, then create your own type and wrap the exception in that.

class DatabaseException extends \RuntimeException { }

...

try {
    return new \PDO ($dsn, $user, $pass);
}
catch (\PDOException $e) {
    throw new DatabaseException("Unable to connect", 0, $e);
}

Now the caller can react specifically to database exceptions, if it chooses. It might try the connection again, for example. When everything is an Exception, on the other hand, exceptions become all but indistinguishable, unless you resort to some ugliness like parsing the exception message (which is really meant for humans, not code).

-
$obj->db->connect() is a violation of what's called the Law of Demeter (LoD). The Law basically says that the only things your code should be talking to are the things it can access without traversing more than one object other than $this*. You should be telling $obj to connect, rather than having to know about $obj->db at all. This keeps it easier to change one class without having to change code in dozens of different places.

* Most languages boil LoD down to "no more than one dot". But that is a bit oversimplified -- particularly in PHP, which doesn't allow for an implicit $this.

StackExchange Code Review Q#49401, answer score: 4