PDO try-catch usage in functions

Question

I'm thinking of using PDO in all of my future webapp. Currently (using what I've learned from SO so far), what I have in my site to handle database connection is a Singleton class like this :

class DB {

    private static $instance = NULL;
    private static $dsn      = "mysql:host=localhost;dbname=mydatabase;";
    private static $db_user  = 'root';
    private static $db_pass  = '0O0ooIl1';

    private function __construct() 
    {

    }
    private function __clone()
    {

    } 
    public static function getInstance() {

     if (!self::$instance)
  {   
      self::$instance = new PDO(self::$dsn, self::$db_user, self::$db_pass);
      self::$instance-> setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  }
     return self::$instance;
    }
}

and another file (functions.php) with content-specific functions looking exactly like this one :

function get_recent_activities ()
{    
    try
    {    
     $db = DB::getInstance();
        // --prepare and execute query here, fetch the result--
     return $my_list_of_recent_activities;
    }
    catch (PDOException $e)
    {
     return "some fail-messages";
    }
}
...

meaning that I have to repeat the try .. catch part in all of the functions.

My questions are :

1. How should I make that more efficient ? (eg. not having to repeat try..catch in all functions, and yet still able to return different "fail-message" on each one)
2. Is this already a good practice ? I'm still new at PDO and OOP (still a lot more to learn), so (as of now), I can't really see any disadvantages or things that can be improved in there.

I'm sorry if that seems unclear or too long. Thanks in advance.

Answer 1

A couple of caveats here are:

• This code is written to take several legacy issues into account such as the database logging and database configuration management.
• I would strongly recommend that you look at an existing solution before building your own. A lot of people think to themselves when they start out that they don't want to use an existing framework or library because they're too big, require too much time to learn, etc., but after having been one of these people I can't state emphatically enough that I am leaving my custom framework and wrapper classes to move to a framework. I am looking to move to Zend but there are a number of excellent choices available.

Oh, I should point out that this point illustrates how one could wrap a single function to handle all of the exception handling for your queries. I don't write try catch blocks almost anywhere else now because the stack trace from the query gives me all of the information that I need to debug the problem and fix it.

Here is my current PDO wrapper class implementation:

class DB extends PDO 
{
    // Allows implementation of the singleton pattern -- ndg 5/24/2008
    private static $instance;

    // Public static variables for configuring the DB class for a particular database -- ndg 6/16/2008
    public static $error_table;
    public static $host_name;
    public static $db_name;
    public static $username;
    public static $password;
    public static $driver_options;
    public static $db_config_path;



    function __construct($dsn="", $username="", $password="", $driver_options=array()) 
    {
     if(isset(self::$db_config_path))
     {
      try 
      {
       if(!require_once self::$db_config_path)
       {
        throw new error('Failed to require file: ' . self::$db_config_path); 
       }
      } 
      catch(error $e) 
      {
       $e->emailAdmin();
      }
     }
     elseif(isset($_ENV['DB']))
     {
      self::$db_config_path = 'config.db.php';

      try 
      {
       if(!require_once self::$db_config_path)
       {
        throw new error('Failed to require file: ' . self::$db_config_path); 
       }
      } 
      catch(error $e) 
      {
       $e->emailAdmin();
      }
     }

     parent::__construct("mysql:host=" . self::$host_name . ";dbname=" .self::$db_name, self::$username, self::$password, self::$driver_options);
     $this->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
     $this->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('QueryStatement', array($this)));

     if(!isset(self::$error_table))
     {
      self::$error_table = 'errorlog_rtab';
     }
    }

    /**
     * Return a DB Connection Object
     *
     * @return DB
     */
    public static function connect()
    {

        // New PDO Connection to be used in NEW development and MAINTENANCE development
        try 
        {
         if(!isset(self::$instance))
         { 
          if(!self::$instance =  new DB())
          {
           throw new error('PDO DB Connection failed with error: ' . self::errorInfo());
          }
         }

         return self::$instance;
        }
        catch(error $e)
        {
         $e->printErrMsg();
        }
    }

    /**
     * Returns a QueryBuilder object which can be used to build dynamic queries
     *
     * @return QueryBuilder
     * 
     */
    public function createQuery()
    {
        return new QueryBuilder();
    }

    public function executeStatement($statement, $params = null, $FETCH_MODE = null)
    {
        if($FETCH_MODE == 'scalar')
        {
         return $this->executeScalar($statement, $params); 
        }


        try {
         try {
          if(!empty($params))
          {
           $stmt = $this->prepare($statement);
           $stmt->execute($params);
          }
          else 
          {
        $stmt = $this->query($statement);
          }
         }
         catch(PDOException $pdo_error)
         {
          throw new error("Failed to execute query:\n" . $statement . "\nUsing Parameters:\n" . print_r($params, true) . "\nWith Error:\n" . $pdo_error->getMessage());
         }
        }
        catch(error $e)
        {
         $this->logDBError($e);
         $e->emailAdmin();
         return false;
        }

     try 
     {
      if($FETCH_MODE == 'all')
      {
       $tmp =  $stmt->fetchAll();
      }
      elseif($FETCH_MODE == 'column')
      {
       $arr = $stmt->fetchAll();

       foreach($arr as $key => $val)
       {
        foreach($val as $var => $value)
        {
         $tmp[] = $value;
        }
       }   
      }
      elseif($FETCH_MODE == 'row') 
      {
       $tmp =  $stmt->fetch();
      }
      elseif(empty($FETCH_MODE))
      {
       return true;
      }
     }
     catch(PDOException $pdoError)
     {
      return true;
     }

        $stmt->closeCursor();

        return $tmp;

    }

    public function executeScalar($statement, $params = null)
    {
        $stmt = $this->prepare($statement);

        if(!empty($this->bound_params) && empty($params))
        {
         $params = $this->bound_params;
        }

        try {
         try {
          if(!empty($params))
          {
           $stmt->execute($params);
          }
          else 
          {
         $stmt = $this->query($statement);
          }
         }
         catch(PDOException $pdo_error)
         {
          throw new error("Failed to execute query:\n" . $statement . "\nUsing Parameters:\n" . print_r($params, true) . "\nWith Error:\n" . $pdo_error->getMessage());
         }
        }
        catch(error $e)
        {
         $this->logDBError($e);
         $e->emailAdmin();
        }

        $count = $stmt->fetchColumn();

        $stmt->closeCursor();

        //echo $count;
        return $count;   
    }

    protected function logDBError($e)
    {
        $error = $e->getErrorReport();

        $sql = "
        INSERT INTO " . self::$error_table . " (message, time_date) 
        VALUES (:error, NOW())";

        $this->executeStatement($sql, array(':error' => $error));
    }
}

class QueryStatement extends PDOStatement 
{
    public $conn;

    protected function __construct() 
    {
        $this->conn = DB::connect();
        $this->setFetchMode(PDO::FETCH_ASSOC);
    }

    public function execute($bound_params = null)
    {
     return parent::execute($bound_params);    
    }
}

Answer 2

Your implementation is just fine, and it'll work perfectly well for most purposes.

It's not necessary to put every query inside a try/catch block, and in fact in most cases you actually don't want to. The reason for this is that if a query generates an exception, it's the result of a fatal problem like a syntax error or a database issue, and those are not issues that you should be accounting for with every query that you do.

For example:

try {
    $rs = $db->prepare('SELECT * FROM foo');
    $rs->execute();
    $foo = $rs->fetchAll();
} catch (Exception $e) {
    die("Oh noes! There's an error in the query!");
}

The query here will either work properly or not work at all. The circumstances where it wouldn't work at all should not ever occur with any regularity on a production system, so they're not conditions that you should check for here. Doing so is actually counterproductive, because your users get an error that will never change, and you don't get an exception message that would alert you to the problem.

Instead, just write this:

$rs = $db->prepare('SELECT * FROM foo');
$rs->execute();
$foo = $rs->fetchAll();

In general, the only time that you'll want to catch and handle a query exception is when you want to do something else if the query fails. For example:

// We're handling a file upload here.
try {
    $rs = $db->prepare('INSERT INTO files (fileID, filename) VALUES (?, ?)');
    $rs->execute(array(1234, '/var/tmp/file1234.txt'));
} catch (Exception $e) {
    unlink('/var/tmp/file1234.txt');
    throw $e;
}

You'll want to write a simple exception handler that logs or notifies you of database errors that occur in your production environment and displays a friendly error message to your users instead of the exception trace. See http://www.php.net/set-exception-handler for information on how to do that.