Which is a better way to connect to a database

This is my current way of connecting to my database

namespace website_project\database;

use PDO;

class ConnectPDO {

    private $pdo_options = [
        /* important! use actual prepared statements (default: emulate prepared statements) */
        PDO::ATTR_EMULATE_PREPARES => false
        /* throw exceptions on errors (default: stay silent) */
        , PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
        /* fetch associative arrays (default: mixed arrays)    */
        , PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC
    ];
    
    public $connection = \NULL;

    public function __construct() {
        $this->connection = new PDO('mysql:host=' . \DATABASE_HOST . ';dbname=' . \DATABASE_NAME . ';charset=utf8', \DATABASE_USERNAME, \DATABASE_PASSWORD, $this->pdo_options);
    }
    
    public function getDatabase() {
        return $this->connection;
    }

}` 

namespace website_project\trivia_game;

use website_project\blog\iCRUD as iCRUD;
use website_project\database\ConnectPDO as ConnectPDO;
use PDO;
use DateTime;
use DateTimeZone;

class Highscores implements iCRUD {

    private $connectPDO = \NULL;
    private $pdo = \NULL;
    protected $query = \NULL;
    public $stmt = \NULL;
    protected $user_id = \NULL;
    public $row = \NULL;
    public $result = \NULL;

    public function __construct(ConnectPDO $connectPDO, $user_id) {
        $this->connectPDO = $connectPDO;
        $this->pdo = $connectPDO->getDatabase();
        $this->user_id = $user_id;
    }

Which uses D.I. (Dependency Injection)

However, I though I would go over to this

namespace website_project\traits;

use PDO;

trait MyPDO {

    private $pdo_options = [
        /* important! use actual prepared statements (default: emulate prepared statements) */
        PDO::ATTR_EMULATE_PREPARES => false
        /* throw exceptions on errors (default: stay silent) */
        , PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
        /* fetch associative arrays (default: mixed arrays)    */
        , PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC
    ];
    protected $db = \NULL;

    protected function db() {
        return $this->db = new PDO('mysql:host=' . \DATABASE_HOST . ';dbname=' . \DATABASE_NAME . ';charset=utf8', \DATABASE_USERNAME, \DATABASE_PASSWORD, $this->pdo_options);
    }

}

and I connect this way
class Blog implements iCRUD {
use \website_project\traits\MyPDO;
private $connectPDO;
private $pdo;
protected $query;
protected $stmt;
protected $sql;
protected $result;
protected $queryParams;
protected $author;
protected $username;
protected $category;
protected $enum;
protected $type;
protected $matches;
protected $vals;
public $categories = ;
protected $row;

    public function __construct() {
        $this->pdo = $this->db();
    }

which uses traits. My main question is “Did I make this less secure?” It’s easier using traits and I feel in the long wrong the classes would be better structured. Though doing the other way isn’t that much more difficult, though to me it seem more clunky (unless I could refine the script some more).

This “where or how should I build my PDO object” decision has nothing to do with security, at least not directly. So, you are ok on that.

I’d say, using the trait for the PDO object building, as you have in the second example with the Blog class, isn’t really the way you want to go about it. To make my concern more clear, I would imagine you would need the PDO object for other calls to the database within one request of your application. Correct? How would you get the PDO object out of your Blog object and into the others to use it? In other words, you shouldn’t really be creating multiple PDO objects with traits within all those classes, which need access to the database. In general, you should only be building one PDO object and passing it to the classes that need it. So, DI is closer to what you need. This will help you later with testing too.

Can you also post the code you have under ConnectPDO?

Scott

First I want to say Thanks for the help.

Here’s the ConnectPDO class:

<?php

namespace website_project\database;

use PDO;

class ConnectPDO {

    private $pdo_options = [
        /* important! use actual prepared statements (default: emulate prepared statements) */
        PDO::ATTR_EMULATE_PREPARES => false
        /* throw exceptions on errors (default: stay silent) */
        , PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
        /* fetch associative arrays (default: mixed arrays)    */
        , PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC
    ];
    
    public $connection = \NULL;

    public function __construct() {
        $this->connection = new PDO('mysql:host=' . \DATABASE_HOST . ';dbname=' . \DATABASE_NAME . ';charset=utf8', \DATABASE_USERNAME, \DATABASE_PASSWORD, $this->pdo_options);
    }
    
    public function getDatabase() {
        return $this->connection;
    }

}

would throwing the following into ConnectPDO prevent duplication?

// Empty clone magic method to prevent duplication:
private function __clone() {
    
}

The way I see it the database connection is a dependency of the model therefore it should be modeled as such using composition not a trait.

Yeah, I switched back to dependency, it was an easy fix CTRL-Z :sunglasses:

To avoid duplicating objects, you’d need to do a bit more. Check out the singleton pattern here:

Also, (and I am being picky here) as a general rule, classes rarely should be verbs or rather, contain them in their names. That is what methods are for. So ConnectPDO could be something like PDOConnection and your method should then be the verb.

PDOConnection::getDatabase();

Also note the paragraph at the end of the explanation. You’ll notice, DI is mentioned and why it is important. :smile:

Scott

I used this in the past:

<?php

namespace website_project\database;

# PDO database: only one connection is allowed. 

use PDO;

class Database {

    private $_connection;
    // Store the single instance.
    private static $_instance;

    // Get an instance of the Database.
    // @return Database: 
    public static function getInstance() {
        if (!self::$_instance) {
            self::$_instance = new self();
        }
        return self::$_instance;
    }

    // Constructor - Build the PDO Connection:
    public function __construct() {
        $db_options = array(
            /* important! use actual prepared statements (default: emulate prepared statements) */
            PDO::ATTR_EMULATE_PREPARES => false
            /* throw exceptions on errors (default: stay silent) */
            , PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
            /* fetch associative arrays (default: mixed arrays)    */
            , PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC
        );
        $this->_connection = new PDO('mysql:host=' . DATABASE_HOST . ';dbname=' . DATABASE_NAME . ';charset=utf8', DATABASE_USERNAME, DATABASE_PASSWORD, $db_options);
    }

    // Empty clone magic method to prevent duplication:
    private function __clone() {
        
    }

    // Get the PDO connection:    
    public function getConnection() {
        return $this->_connection;
    }

}

I don’t like using static methods/functions.
So if I do it the D.I. way then I don’t have to worry duplicate objects that is how I take the last read paragraph to read. Am I correct?
Thanks for the help again.

P.S. I agree it should be PDOConnection instead, I sometimes get lazy when I don’t name my variables very well. :grinning:

Yeah, singletons are bad because an object should not control how it is instantiated by the caller.

If you need a single instance then the creation of the object should be moved outside - for example a factory class. Also, most (maybe all?) dependency injection containers support creation of shared instance objects. In both of these cases you can continue using dependency injection.

I am no expert, but I know you are right. Don’t use static methods, if you don’t absolutely have to.

Though, to make sure only one instance of an object is created, usage of a static class/method is most likely necessary ( I don’t know of any other way, but if someone else does, please do explain). That being said, you can “hide” the static single instantiation method too. (and why I linked to PHPTheRightWay: see factory pattern for example and as Lemon_Juice mentioned). Theoretically, with the factory pattern, you could even extend your PDO connection system to have multiple single connections to different databases.

As for DI. No, DI alone doesn’t stop the duplication of an object. To enforce single object instantiation, you will still have to have some sort of code set up to check if the object is already created or not. (and also why I referred to PHPTheRightWay’s singleton pattern). :wink:

Scott

1 Like

I believe I found a nice factory method where I think I can easily modify into my current PDOConnection class, thus keeping D.I. along in the process. Thanks for the help once again. At least I’m learned something (I hope). :wink:

Can you please share this method? I looking for the good factory implementation for PDO-based CRUD too.

If you use a factory then static methods are not necessary. The single instance is held in a private property of the factory. Of course, whenever you need to get the database object instance you need to have the instance of the factory and use the factory to grab the database object. This might get tedious to pass the factory object everywhere so it’s a good idea to have all object creation delegated to factories - then those factories which create objects dependent on the database will require the database factory as a dependency. As a result we get rid of the (global) new keyword in most of our code (which is a good thing) - new is only used in factories and the classes of our application simply require needed objects via dependency injection.

A similar concept is dependency injection container - I haven’t used it much but to me it is a sort of automated factory. If there are many objects to create in a project it may be tedious to create a factory for each object (even though such a factory is very simple) - then a DIC can create those objects automatically based on a set of conventions - most often according to argument type hints.

1 Like

Now that you say that, it hits me. Yes. But, that doesn’t necessarily mean you only create one instance. There still needs to be code in the factory that stops (re)instantiation`of the connection object. Right?

Scott

Yes, of course. The factory decides whether to create and use only one instance or more. A simplest example of a factory that returns only one instance:

class DbFactory {
    private $db;
    
    public function getDb() {
        if (!$this->db) {
            $this->db = new DbConnection();
        }
        
        return $this->db;
    } 
}
1 Like

Very good. And, theoretically, if we were using multiple connections (for whatever reason), we could make $db and array of objects with certain ids or types or whatever for the connection identification.

Scott

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.