Error in Implementing singleton pattern

Question

I am trying to implement singleton pattern and i am getting the following error

Fatal error: Access level to Database::__construct() must be public (as in class PDO) in /config/database.php on line 29

<?php

class Database extends PDO
{
    private static $instance;

    private function __construct()
    {

            return parent::__construct(
                    "mysql:host=localhost;dbname=live",
                    "root",
                    "root"
            );

    }

    public function getInstance() {
        if(self::$instance === null) {
            self::$instance = new Database();
        }
        return self::$instance;
    }

    public function __clone() 
    {
        die(__CLASS__ . ' class cant be instantiated. Please use the method called getInstance.');
    }
}


$mySingleton = Database::getInstance();

var_dump($mySingleton);

?>

This question appears to be off-topic because error message explains the source of problem. — tereško, Sep 19 '13 at 00:05

Jeremy Harris · Answer 1 · 2012-03-11T16:05:32.550

By declaring the __construct() function as private like private function __construct() you are effectively disallowing PHP from automatically calling it upon object creation.

Instead, you should always have your __construct() as well as other magic methods declared public.

public function __construct() 
{
   // Now PHP can access me
}

As far as making your Database class follow a singleton pattern, it doesn't make sense to extend a class that doesn't (i.e. PDO). Instead, do something like this:

<?php

class Database
{
    private static $instance;

    public function __construct()
    { 
        // Check to see if static PDO instance
        // has already been created, so we create only one (singleton)
        if(!self::$instance)
        {
            self::$instance = new PDO(
                "mysql:host=localhost;dbname=live",
                "root",
                "root"
            );
         }
    }

    public function getInstance() {
        if(self::$instance === null) {
            self::__construct();
        }
        return self::$instance;
    }

    public function __clone() 
    {
        die(__CLASS__ . ' class cant be instantiated. Please use the method called getInstance.');
    }
}


$mySingleton = Database::getInstance();

var_dump($mySingleton);

?>

`if(self::$instance === null) {self::__construct();}`, `self::__construct();`this is wrong because the construct can't be static. — Lion King, Apr 24 '14 at 10:12
Since you are no more extending PDO class you can set constructor to private. Also insted of... "if(self::$instance === null)" it will be more apropriate to use: "if(!(self::$_instance instanceof PDO))" The only way of obtaining instance of PDO object should be by running call to static method getInstance(). In your code this method is not static. It should be "public static function getInstance(){...}" Then in your code you only use "Database::getInstance()" — DevWL, Mar 07 '16 at 14:48

score 3 · Answer 2 · edited Dec 18 '17 at 15:33

3

As PDO's __construct() function is public, you cannot extend it with a private __construct() function.

So a "real" singleton is not possible.
You have to set public function __construct().

edited Dec 18 '17 at 15:33

yodabar

4,651
2
33
38

answered Mar 11 '12 at 15:50

Frederick Behrends

3,075
23
47

score 3 · Accepted Answer · answered Mar 11 '12 at 15:52

3

You can't change the Access level for a override method.

Instead of extending PDO, you could just have a PDO instance in Database. Composition is more flexible than inheritance.

answered Mar 11 '12 at 15:52

xdazz

158,678
38
247
274

score 0 · Answer 4 · edited Feb 01 '18 at 23:19

You should

1) disable constructor by setting it to private.

2) create single PDO object by calling the static method only. Static method have to return the instance of PDO object.

In Silex or Symfony you will have to prefix class name with "\" or use "use \PDO;". Which means nothing more that this is a global class.

Ps. if you set the __constructor to public and use return functino note that it will not generate any exception or warning but you will get an class object returned and not actual value of return statement.

So $db = new Database() would return object of class Database. Then from there you would have to access your PDO using class method. $pdo = $db->getInstance() Which is not the right way to build proper singleton.

If you interested in reading more about singletons pros and cons and some use cases read this Best practice on PHP singleton classes, you will find more information about this pattern design.

/**
 *  Singleton pattern
 */
class Database
{

    /**
     *  holds the only PDO instance
     */
    private static $_instance;

    /**
     *  private __constructor not accesible
     */
    private function __construct()
    {

        self::$instance = new PDO(
            "mysql:host=localhost;dbname=live",
            "root",
            "root"
        );

    }

    /**
     *  clone will not duplicate object
     */
    public function __clone() 
    {

        die(__CLASS__ . ' class cant be instantiated. Please use the method called getInstance.');

    }

    /**
     *  the only public function to access or create PDO object
     */
    public static function getInstance()
    {

        if(!self::$_instance instanceof PDO){
            new self;
        }

        return self::$_instance;

    }

}

score 0 · Answer 5 · answered Dec 18 '17 at 16:00

The need to have different access level between parent and child constructors is perfectly lecit, apart from technical difficulties involving the PHP source code.

In fact, it has been filed as a bug (#61970).

In this commit made on march 2017, if I am not wrong, the developer says it has been closed:
http://git.php.net/?p=php-src.git;a=commitdiff;h=5324fb1f348f5bc979d9b5f13ac74177b73f9bf7

I use PHP 7.0.24 released on july, but I still see it has not been fixed.

Error in Implementing singleton pattern

5 Answers5