Try/catch block in PDO wrapper

Question

I have written a PDO wrapper and a class file like a model. It's looking good so far, but I'm just confuseed on where I should put the try/catch block. Would it be better to place it in the PDO wrapper (pdoDatabase.php), mobile.php (model) or test.php?

pdoDatabase.php

class pdoDatabase {

public $db;
private $sth;
private static $instance;
private $error;

private function __construct() {
    $dsn = "mysql:dbname=" . config::read("db.database") . ";host=localhost";
    $username = config::read("db.username");
    $password = config::read("db.password");
    $this->db = new PDO($dsn, $username, $password);
    $this->db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}

public static function getInstance() {
    if (!isset(self::$instance)) {
        self::$instance = new static();
    }
    return self::$instance;
}

public function fetchData($sql, $params = array(), $fetchMode = 'all') {
    // Should try...catch block be here?
    try {
        $this->sth = $this->db->prepare($sql);
        if (count($params) > 0)
            $this->setBind($params);

        $this->sth->execute();
        $fetch = ($fetchMode == 'single') ? "fetch" : "fetchAll";
        return $this->sth->$fetch();
    } catch (PDOException $e) {
        $this->error = array(
            'status' => 'error',
            'message' => $e->getMessage()
        );
        return false;
    }
}

public function setBind($params) {
    if (is_object($this->sth)) {
        foreach ($params as $key => $value) {
            $this->sth->bindValue(":$key", $value);
        }
    }
}

public function error() {
   return $this->error;
}
public function beginTransaction() { }
public function commit() { }
public function rollBack() { }

mobile.php

class Mobile {

    public $db;

    public function __construct() {
        $this->db = pdoDatabase::getInstance();
    }

    public function findPhoneAffiliate($phoneId = null, $affiliateId = null, $affiliatePhoneId = null) {
     // or should try...catch block be here?
        $bindAr = array();
        if ($phoneId !== null)
            $whereAr[] = "phone_id = " . $phoneId;

        if ($affiliateId !== null)
            $whereAr[] = 'affiliate_id = ' . $affiliateId;

        if ($affiliatePhoneId !== null) {
            $whereAr[] = 'affiliate_phone_id = :affiliate_phone_id';
            $bindAr['affiliate_phone_id'] = $affiliatePhoneId;
        }

        $where = count($whereAr) > 0 ? 'WHERE ' . implode(' AND ', $whereAr) : '';

        $sql = "SELECT * FROM phone_affiliate $where";
        return $this->db->fetchData($sql, $bindAr, 'single');
    }
}

test.php

$mobileResult = $mobile->findPhoneAffiliate(null, 'f', "ASEF33");
if ($mobileResult) {
    // Data Found!
    print_r($mobileResult);
} else {
    if ($mobile->db->error()) {
        print_r($mobile->db->error());
        exit();
    }
    if ($mobile->db->rowCount() == 0)
        echo "Data Not Found";
}

As you can see, it will need to check if there any error or no data found in the else statement. Is this how it should be done? If now, what can be improved to reduce the code from the test.php sample?

Answer 1

I'll probably revisit this later, but for now:

---

The exception catching should be done at either the model or model-using layer. I would probably let the exceptions bubble up through the model unless you want the model to be fairly high level.

For the sake of abstracting the DB layer away from the model layer, I might consider wrapping the PDOException in some kind of model exception(s). Exception wrapping is typically an anti-pattern, but it would keep your consuming code from needing to know that the models are all dependent on PDO. (Thus allowing you to change PDO to something else if you ever wanted to -- basically it's a weird instance of separation of concerns and hiding implementation details.)

I'm starting to ramble, but what I'm trying to convey is that your consuming code needs to know something went wrong. If you handle the exception in your DB class, you model has no (pleasant) way to know that something went wrong. If you handle the exception in your model, the level above the model has no (pleasant) way to know something went wrong. Basically your consuming code needs to know something happened, and for that to be the case, you need exceptions to come out of the model. (An alternative would of course be returning a Boolean or something along those lines. That tends to not fit the OO paradigm very well though, and PDO sometimes uses Boolean false when something isn't an error.)

---

Your pdoDatabase class shouldn't be a singleton.

---

$this->db = pdoDatabase::getInstance(); you pass the pdoDatabase instance as a parameter instead of coupling your code to this static method.

Dependency Injection

---

There's a few other things that stuck out to me, but I'm out of time now ;(. Will come back later.

---

Exception Catching

It should be extremely rare for an exception to actually happen with your Mobile.

Consider the sources of exceptions: connection drops, invalid syntax, unknown column/table/etc, so on. Basically there's different layers of exceptions. There's the connection oriented ones, and the SQL oriented ones. The SQL oriented ones should never happen. By the time your code goes into production, you should be certain that none of your queries have invalid syntax, reference non-existing entities, so on.

This leaves connection problems as the only source of exceptions (or perhaps MySQL related hiccups, though those should be pretty rare).

Because of this rarity, I would be tempted to basically ignore any exceptions.

Even if you did handle the exceptions, what are you going to do with them?

try {
    $affs = $mobile->findPhoneAffiliate(5);
} catch (SomeExceptionClass $ex) {
    //There's not really a good way to recover from this, so you might as well
    //show some kind of "Oops, something broke." page.
    //You could then log the exception behind the scenes and make sure the end-user
    //sees nothing sensitive.
}

What I would do in this situation is just let the exception bubble up to the very top. I would have a giant try/catch at the top layer though that intercepts any uncaught exceptions and renders some kind of error page.

How you do this will highly depend on the structure of your website, but assuming you're doing something MVC-ish, it should be fairly simple. In the catch block, just kill the current dispatch, and instead dispatch a request for the exception page.