0
\$\begingroup\$

I created registration system with PHP. I have User class that will handle all the input and insert the user input into database, and i created Database class with Singleton pattern that will connect to database. After user submited his data, new User object is created.

This is my Database class with singleton patter:

class Database {
    private static $instance = null;
    private $conn;

    private function __construct() {
        $dsn = "mysql:host=" . DB_HOST . ";dbname=" . DB_NAME;
        $this->conn = new PDO($dsn, DB_USER, DB_PASS);
    }
    private function __clone() {
        return false;
    }
    private function __wakeup() {
        return false;
    }
    public static function getInstance() {
        if(!self::$instance) {
            self::$instance = new Database();
        }
        return self::$instance;
    }
    public function getConnection() {
        return $this->conn;
    }
}

And this is my User class:

class User {

    public $errors = array();
    protected $username;
    protected $pw1;
    protected $pw2;
    protected $mail;
    private $db;
    private $conn;

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

I didn't copied rest of the User class code, because my question is: Is this a good way to create singleton pattern for db connection, and a good way to use it in other classes? I'm fairly new to PHP, and i understand good amount of writing the PHP code, but what i lack is practical knowledge, what are common practices and how to structure the code.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

For reasons laid out in this stackoverflow question the singleton design pattern is not good match for PHP!

Clone Prevention

The advice from the accepted answer to the above stackoverflow question gives is to stop callers from cloning your object by adding a magic method like so:

public function __clone() {
    throw new Exception("Can't clone a singleton");
}

Your code will return false. false can be ignored / overlooked by programmers quite easily so I would stick to throwing an exception which is more explicit!

The Database Class

According to the php manual

The intended use of __wakeup() is to reestablish any database connections that may have been lost during serialization and perform other reinitialization tasks.

Seeing as your class doesn't do that I can't see the value in the __wakeup method at all. There is a quote that is something along the lines of "don't write code you don't need now, because you won't in the future".

Chaining methods

As your getInstance method just returns an instance of the Database class and the method getConnection is public you can chain the calls like so:

$this->db = Database::getInstance()->getConnection();

PDO Exceptions

Make sure when setting up your PDO error mode to throw exceptions; this will prevent instances of trying to chain functions based on objects that fail to setup correctly.

Conclusion

If its not impractical too I would recommend (based on personal biases) that you drop the singleton pattern and move towards dependency injection simply because its easier to test and easier to get your head around!

\$\endgroup\$
1
  • \$\begingroup\$ Thanks a lot for answer! I will move to dependency injection. \$\endgroup\$
    – bici
    Commented Aug 7, 2018 at 22:50

Not the answer you're looking for? Browse other questions tagged or ask your own question.