Connection class

Question

I've built a Connection class that uses a PDO connection, but it looks messy and I'm sure there are ways of improving it. Currently I only want 1 instance of the connection (singleton).

<?php
namespace AQEConnect {

    interface ICredentials {
        function Username();

        function Password();
    }

    interface IDbProvider {
        function Options();
    }

    class DatabaseException extends \Exception {
    }

    abstract class SqlConnection implements IDbProvider {
        protected $_connection_type;
        private $_connection = null;
        private $_connection_string;
        private $_credentials;

        public function __construct($connection_string, ICredentials $credentials) {
            $this->_connection_string = $connection_string;
            $this->_credentials = $credentials;
        }

        public function get() {
            $this->_connect();

            return $this->_connection;
        }

        private function _connect() {
            if ($this->_connection === null)
                try {
                    $this->_verifyIsSet($this->_connection_type, $this->_connection_string,
                        $this->_credentials->Username(), $this->_credentials->Password(), $this->Options());

                    $this->_connection = new \PDO($this->_connection_type . $this->_connection_string,
                        $this->_credentials->Username(), $this->_credentials->Password(), $this->Options());

                } catch (\PDOException $exception) {
                    $this->_connection = null;

                    throw new DatabaseException('Database connection exception', $exception);
                }
        }

        protected function _close() {
            $this->_connection = null;
        }

        /**
         * @return bool Whether the class currently has a connection.
         */
        public function hasConnection() {
            return $this->_connection !== null && $this->_connection instanceof \PDO;
        }

        private function _verifyIsSet() {
            foreach (func_get_args() as $arg)
                if (empty($arg))
                    throw new DatabaseException('Required database variable not set.');
        }

        public function __destruct() {
            $this->_close();
        }

        /**
         * @return string Current Connection class details.
         */
        public function __toString() {
            return $this->hasConnection() ? 'Connected to: ' . DBNAME . '.' : 'No Connection.';
        }
    }

    class MysqlConnection extends SqlConnection {
        public function __construct($connection_string, ICredentials $credentials) {
            $this->_connection_type = 'mysql:';

            parent::__construct($connection_string, $credentials);
        }

        public function Options() {
            return array(\PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',
                \PDO::ATTR_ERRMODE => \PDO::ERRMODE_SILENT);
        }
    }
}

Its called like this:

class DbCredentials implements ICredentials {
    public function UserId() {
        return 'test';
    }

    public function Password() {
        return '12345';
    }
}

class foo
{
    private static $_connection = null;

    private static function _connect() {
        if (self::$_connection === null)
            self::$_connection = new MysqlConnection('host=localhost;port=1111;dbname=test', new DbCredentials());
    }
}

EDIT: Updated my connection class to an updated version (Thanks to Peter Kiss' answer), I'm still looking for any help from anyone who can suggest any modifications!

Answer 1

Basic mistakes

• Forget static, it's not object oriented, it's ugly and unreadable

• Forget constans in classes, inject them instead (global hyperspace is bad, untestable, uncontrolable)

Generic approach

<?php

interface ICredentials {

    function UserID();

    function Password();
}

interface IDbProvider {

    /**
     * 
     * @return array Driver specific options for PDO
     */
    function GetOptions();
}

class MySQL implements IDbProvider {

    public function GetOptions() {
        return array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8');
    }

}

/*abstract*/ class SqlConnection {

    /**
     *
     * @var \PDO
     */
    private $_connection;
    private $_connectionString;

    /**
     *
     * @var \ICredentials 
     */
    private $_credentials;

    /**
     *
     * @var \IDbProvider
     */
    private $_options;

    public function __construct($connectionString, \ICredentials $credentials, \IDbProvider $options) { //or protected, especially if abstract class
        $this->_connectionString = $connectionString;
        $this->_credentials = $credentials;
        $this->_options = $options;
    }

    /**
     * 
     * @return \PDO
     */
    public function Connection() { //or protected, especially if abstract class
        if ($this->_connection == NULL) {
            $options = $this->_options->GetOptions();
            $options[\PDO::ATTR_ERRMODE] = \PDO::ERRMODE_EXCEPTION;

            $this->_connection = new \PDO($this->_connectionString, $this->_credentials->UserID(), $this->_credentials->Password(), $options);
        }

        return $this->_connection;
    }

    protected function _close() {
        if ($this->_connection != NULL) {
            $this->_connection = NULL;
        }
    }

    public function __destruct() {
        $this->_close();
    }

}

Usage #1

(First you have to implement IDbCredentials.)

$connection = new SqlConnection('mysql:host=localhost;dbname=testdb', $credentialsInstance, new MySQL());

Usage #2 as abstract

The instantiating is the same, in this current example the options aren't injected (i would do this in a generated db specific object context class but in a general DbConnection as in the example).

class DbConnection extends SqlConnection {

    public function __construct($connectionString, \ICredentials $credentials) {
        parent::__construct($connectionString, $credentials, new MySQL());
    }

    public function Execute($something /* ... parameters ... */) {
        try {
            $this->Connection()->beginTransaction();

            /* something */

            $this->Connection()->commit();
        } catch (\PDOException $pdoExc) {
            if ($this->Connection() != NULL && $this->Connection()->inTransaction()) {
                $this->Connection()->rollBack();
            }

            throw $pdoExc;
        }
    }

}

$connection = new DbConnection('mysql:host=localhost;dbname=testdb', $credentialsInstance);

$connection-Execute("INSERT INTO ...", array(/* params */));

• Easy to read
• Easy to understand
• Easy to test
• Easy to reuse
• Maybe __clone can be implemented :)

Edit

You can create a derived class which constructor accepts only one object instance which is holding all the information what are needed to establish the connection. (The implementation can be read the configuration values from XML or any other source.)

Answer 2

This is a little bit of a reworked class based on your code example. Some things to keep in mind while writing your version:

• Is this testable? Currently it is not; the db config is assumed, the toString is assuming a db credential ( DBNAME ), etc.

• Your db connectivity is coupled to this class. What would happen if you wanted to use this for another application?

• Your db connectivity is hinged on the fact that the assumption is made there is a config file. What will happen if there is no config.php? The path to the config is hardcoded to be the root of this DIR. What happens if this script is moved?

• Declaring database connection parameters as global definitions is generally not a good idea. Having PORT/HOST/DBNAME/USERNAME/PASSWORD out in the global space ( assuming you are defining them in the config.php ) is a security risk.

Anyhow, here is a slighly reworked class.

namespace AQEConnect;

class Connection
{
/**
 * @var \PDO|null
 */
private static $connection;

/**
 * Return whether or not there is a current database connection
 *  or not.
 * 
 * @return string
 */
public function __toString()
{
    if (!is_defined('DBNAME')) {
        self::extractConfiguration();
    }

    return (!($connection instanceof \PDO)) ? 
        'No Connection' : 'Connected to: ' . DBNAME;
}

/**
 * Based on a config static location, extract the necessary database
 *  configuration from the required file.
 */
public static function extractConfiguration()
{
    // Include config to local database credentials for connecting
    $configPath = dirname(__DIR__) . '/config.php';
    if (!file_exists($configPath)) {
        trigger_error('Exception: Failed to find database connection configuration.');
        die;
    }

    require_once dirname(__DIR__) . '/config.php';
}

/**
 * Connects to the database through the PDO abstraction layer
 *  given a set of assumed database credentials. It will attempt
 *  to connect 5 times before it fails.
 *
 * @return boolean
 */
public static function connect()
{
    // Before we attempt any of this logic make sure that we do
    //  not currently have a connection.
    if (self::$connection) {
        return true;
    }

    // Make sure the config credentials are loaded.
    self::extractConfiguration();

    $attemptsLimit = 5;
    $attempt       = 0;
    while ($attempt < $attemptsLimit) {
        self::$connection = new PDO(
            'mysql:host=' . HOST . ';port=' . PORT . ';dbname=' . DBNAME, 
            USERNAME, 
            PASSWORD, 
            array(
                PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8'
            )
        );

        if (self::$connection) {
            break;
        }

        $attempt++;
    }

    // If we failed to get a connection at this point we want to go ahead
    //  and kill the script and throw an error.
    if (!self::$connection) {
        $trace = debug_backtrace();
        trigger_error(
            'Exception: [' . $exception . '] in ' . $trace[0]['file'] .
                ' on line ' . $trace[0]['line'], E_ERROR
        );
        die; 
    }

    return true;
}

/**
 * Returns a boolean result which represents whether or not
 *  an open connection is currently present.
 *
 * @return boolean
 */
public static function hasConnection()
{
    return is_null(self::$connection);
}

/**
 * Returns an instance of the set database instance. If the
 *  connection is null, it will attempt to connect.
 * 
 * @return \PDO
 */
public static function getInstance()
{
    if (!self::$connection) {
        self::connect();
    }

    return self::$connection;
}
}