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

Should a PDO object be passed to classes and procedural functions as a parameter?

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

Problem

I've been trying to figure out the best way to manage database connections for a while now and am still unsure of how I should be doing things. I know there are a lot of similar questions on here, but I've read many of them and am still confused.

Some answers on here say singletons should be used, but it seems a larger percentage of answers say not to use singletons because they make debugging much harder.

My current method is to create a PDO object and then pass it as a parameter:

/**
 * Creates and returns PDO object
 *
 * @return PDO
 */
function createPDO() {
    // MySQL connection
    $dbUser = '';
    $dbPass = '';
    $dbName = '';

    try {
        $db = new PDO("mysql:dbname=$dbName;charset=utf8", $dbUser, $dbPass, array(PDO::ATTR_EMULATE_PREPARES => false, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION));
    }
    catch (PDOException $e) {
        errorHandler(0, $e->getMessage(), $e->getFile(), $e->getLine(), print_r($e, true), 'sql');
        die('SQL Error, terminating script.');
    }

    return $db;
}

/**
 * Makes a PDO query
 *
 * @param @var $stmt The PDO statement
 * @param array $array The array of PDO statement parameters
 */
function makeQuery($stmt, $array) {
    try {
        $stmt->execute($array);
    }
    catch (PDOException $e) {
        errorHandler(0, $e->getMessage(), $e->getFile(), $e->getLine(), print_r($e, true), 'sql');
        die('SQL Error, terminating script.');
    }
}


Then in my scripts I'll use the object like this:

$db = createPDO();

$stmt = $db->prepare('SELECT ....')
makeQuery($stmt, array(':boundParameter' => '..'));
$result = $stmt->fetch(PDO::FETCH_ASSOC);


If I instantiate new classes, or call procedural functions, I pass $db as a parameter:

$myClass = new myClass($db);


Is this a good way of doing things, or is there a better way?

Solution

Let's start with the review, and then talk about correctness of the approach.

-
Pass things by parameters - What do you do if your credentials change? Don't you want to reuse the function? Pass the credentials as parameters to the createPDO function, or better yet, the DSN directly:

function createPDO($dsn, $user, $password)


-
Add good practice defaults to PDO instance - Use $pdo->setAttribute() to set PDO::ATTR_ERRMODE to PDO::ERRMODE_EXCEPTION and PDO::ATTR_EMULATE_PREPARES to false.

-
Don't handle exceptions in your function - You are creating hidden dependencies (You now depend on errorHandler to exist somewhere in your application). Also, you don't know what you want to do if you fail creating the PDO instance. In this app you may want to kill the script, in another you may want to fall back to the filesystem. It's not your function's job to know!

-
Typehint what you can - Your $stmt variable is a PDOStatement object. You should hint it in the function's signature. Your $array is an array.

function makeQuery(PDOStatement $stmt, array $params)


Approach wise, you should ask yourself, What in my application needs a PDO object?

If the rest of your application is procedural, there's no point in even what little abstraction you tried to give here. However, if you do have some sort of other object which uses the PDO connection, then using lazy loading is a good practice to follow:

$pdoProvider = function() {
    //Do all PDO instantiation logic here
};
$mapperFactory = new MapperFactory($pdoProvider);


By passing in a function instead of the actual object, you can delay the creation of the object until it's first needed (which may not even happen, saving you the connection!)

In the MapperFactory:

class MapperFactory {
    private $provider;
    private $pdoInstance;
    public function __construct(callable $provider) {
        $this->provider = $provider;
    }
    public function create($mapper) {
        if ($this->pdoInstance !instanceof PDO) {
            $this->pdoInstance = call_user_func($this->provider); //Call the provider, get the PDO instance
        }
        //By this point, $this->pdoInstance has a PDO instance for sure.
    }
}


For more information, See this answer on Stack Overflow.

Code Snippets

function createPDO($dsn, $user, $password)
function makeQuery(PDOStatement $stmt, array $params)
$pdoProvider = function() {
    //Do all PDO instantiation logic here
};
$mapperFactory = new MapperFactory($pdoProvider);
class MapperFactory {
    private $provider;
    private $pdoInstance;
    public function __construct(callable $provider) {
        $this->provider = $provider;
    }
    public function create($mapper) {
        if ($this->pdoInstance !instanceof PDO) {
            $this->pdoInstance = call_user_func($this->provider); //Call the provider, get the PDO instance
        }
        //By this point, $this->pdoInstance has a PDO instance for sure.
    }
}

Context

StackExchange Code Review Q#54092, answer score: 3

Revisions (0)

No revisions yet.