HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpCritical

Class for reducing development time

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
reducingtimedevelopmentforclass

Problem

There are many PHP PDO classes out there, agreed. However I find they do not allow for flexibility. So I created one that helps reduce development time as little as it may be but it does the job (maybe apart from the disconnect part, but it allows to trace whether database is connected via $database->isConnected). Can you please point out any flaws and any possible improvements?

```
isConnected = true;
try {
$this->datab = new PDO("mysql:host={$host};dbname={$dbname};charset=utf8", $username, $password, $options);
$this->datab->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$this->datab->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC);
}
catch(PDOException $e) {
$this->isConnected = false;
throw new Exception($e->getMessage());
}
}
public function Disconnect(){
$this->datab = null;
$this->isConnected = false;
}
public function getRow($query, $params=array()){
try{
$stmt = $this->datab->prepare($query);
$stmt->execute($params);
return $stmt->fetch();
}catch(PDOException $e){
throw new Exception($e->getMessage());
}
}
public function getRows($query, $params=array()){
try{
$stmt = $this->datab->prepare($query);
$stmt->execute($params);
return $stmt->fetchAll();
}catch(PDOException $e){
throw new Exception($e->getMessage());
}
}
public function insertRow($query, $params){
try{
$stmt = $this->datab->prepare($query);
$stmt->execute($params);
}catch(PDOException $e){
throw new Exception($e->getMessage());
}

Solution

Personally, I must say that close to all PDO derived/based classes I've seen so far suffer from the same problem: They are, essentially, completely pointless.
Let me be clear: PDO offers a clear, easy to maintain and neat interface on its own, wrapping it in a custom class to better suit the needs of a particular project is essentially taking a decent OO tool, and altering it so that you can't reuse it as easily in other projects. If you were to write a wrapper around MySQLi, I could understand the reasoning, but PDO? No, I'm really struggeling to understand the logic behind that unless:

You were to write a table mapper class to go with it, that establishes a connection between the tables you query, and the data models into which you store the data. Not unlike how Zend\Db works.

MySQL is, as you well know, not as flexible as PHP is in terms of data-types. If you are going to write a DB abstraction layer, common sense dictates that layer reflects that: it should use casts, constants, filters as well as prepared statements to reflect that.

Most mature code out there also offerst an API that doesn't require you to write your own queries:

$query = $db->select('table')
            ->fields(array('user', 'role', 'last_active')
            ->where('hash = ?', $pwdHash);


These abstraction layers often (if not always) offer another benefit, they build the queries for you, based on what DB you're connection to. If you're using mysql, they'll build a MySQL query, if you happen to switch to PostgreSQL, they'll churn out pg queries, without the need to rewrite thousands of queries. If you want to persist and write your own abstraction layer, make sure you offer something similar, too. If you don't, you're back to square one: embarking on a labour intensive, pointless adventure that won't be worth it.

An alternative approach is to extend the PDO class. Again, this has been done before and is, in theory, perfectly OK. Although, again this might be personal, it does violate one principle which is upheld by many devs I know: Don't extend, or attempt to change an object you don't own. PDO is a core object,so it's pretty clear you don't own it.

Suppose you were to write something like:

class MyDO extends PDO
{
    public function createProcedure(array $arguments, $body)
    {
        //create SP on MySQL server, and return
    }
}


And lets assume that, after some serious debugging, and testing, you actually got this working. Great! But then what if, some time in the future, PDO got its own createProcedure method? it'll probably outperform yours, and might be more powerful. That, in itself isn't a problem, but suppose it's signature were different, too:

public function createProcedure (stdClass $arguments)
{
}


That would mean you either have to ditch your method, and refactor your entire code-base to sing to the tune of PDO's latest and greatest hit, or you'd have to alter your method to:

public function createProcedure(array $arguments, $body)
{
    $arguments = (object) $arguments;//create stdClass
    //parse $body and:
    $arguments->declare = $body[0];
    $arguments->loops = (object) array('loop1' => $body[1], 'loop2' => $body[2]);
    $arguments->body = (object) array(
        'main' => $body[3],
        'loop1_body' => $body[4],
        'loop2_body' => $body[5]
    );
    return parent::createProcedure($arguments);
}


That would mean that, for all code you wrote, you're actually having to call 2 methods, turning your once so clever createProcedure method into dead weight. So what, you might say? Well, don't think you're out of the woods just yet, because This alternative method above is illegal, it can't be written, it can't work, it shouldn't work, it's just all shades of wrong, here's why:

The Liskov principle states that a child (the class extending PDO) may not alter the signature of inherited methods if those alterations constitute a breach of contract, meaning the expected types (type-hints) may not be stricter than or different to the types defined in the parent (ie: array vs stdClass is not allowed). Additional arguments are allowed, provided they're optional.

If the PDO method itself takes but a single argument of the type stdClass, then your child class may only add optional arguments, and should either drop the type-hint, or uphold it (ie: hint at stdClass, which would break all existing code), or don't hint at all (which is as error-prone as it gets).

What's more, after a couple of months, people might use third party code (frameworks), that rely on the createProcedure method, and pass it an instance of stdClass. You'll have to change your method again, to the vague, and error prone signature:

```
public function createProcedure($arrOrObject, $body = null)
{
if ($arrOrObject instanceof stdClass)
{
return parent::createProcedure($arrOrObject);
}
if ($body === null)
{
//What now??
}
//par

Code Snippets

$query = $db->select('table')
            ->fields(array('user', 'role', 'last_active')
            ->where('hash = ?', $pwdHash);
class MyDO extends PDO
{
    public function createProcedure(array $arguments, $body)
    {
        //create SP on MySQL server, and return
    }
}
public function createProcedure (stdClass $arguments)
{
}
public function createProcedure(array $arguments, $body)
{
    $arguments = (object) $arguments;//create stdClass
    //parse $body and:
    $arguments->declare = $body[0];
    $arguments->loops = (object) array('loop1' => $body[1], 'loop2' => $body[2]);
    $arguments->body = (object) array(
        'main' => $body[3],
        'loop1_body' => $body[4],
        'loop2_body' => $body[5]
    );
    return parent::createProcedure($arguments);
}
public function createProcedure($arrOrObject, $body = null)
{
    if ($arrOrObject instanceof stdClass)
    {
        return parent::createProcedure($arrOrObject);
    }
    if ($body === null)
    {
        //What now??
    }
    //parse body
}

Context

StackExchange Code Review Q#29362, answer score: 70

Revisions (0)

No revisions yet.