6

All the singleton patterns I have seen use a reference to the object to determine if the object has been instantiated. However, if I am using a singleton to guarantee only one db connection, why not use the db connection resource link to do this? Here is the code I am using. (PS: it works fine). I use the comment to be able to search for my classes easily. 

/*one*/ class one { public static $db; private function __construct() { self::$db=new mysqli(DB_HOST, DB_USER, DB_PASS, DB_DATABASE); } public static function get() { if(self::$db==NULL) { new self(); } return self::$db; } }
Improve this question
4
  • 1
    Is still singleton. Nothing unusual there, I’d say. Commented Sep 10, 2011 at 0:07
  • 2
    Errr... Nobody see that the get method returns a different object? Commented Sep 10, 2011 at 0:10
  • 1
    @Clive which ingenuity do you mean? he didn't invet the singleton pattern. am I missing something? Commented Sep 10, 2011 at 0:15
  • 1
    I like the code comments to this class :) Commented Sep 10, 2011 at 8:24

5 Answers 5

Reset to default
5

In PHP, a constructor does not return.

So your get method returns a one object, the first time it's called, then a mysqli object. Probably not what you want.

if( self::$_db == NULL ) { return new self(); // Here you return an object of class one } else { return self::$_db; // Here you return an object of type mysqli }

If you want to return the mysqli object, you do not need a singleton, as there is no need to create an instance of an object that's only here to return an instance of another object.

The registry pattern would be better in such a case.

If you need to provide methods (a wrapper for your DB object), then create a real singleton.

EDIT

I checked your updated code. Now you return always the mysqli instance. But you do not need to instantiate your own object. That's completely useless...

If you really want to go with your kind of pattern, as golden said, in your static instance, checks if self::db is NULL. If yes, creates the mysqli instance, and assign it to self::db. Then returns it.

public static getDatabaseInstance() { if( self::$_db == NULL ) { self::$_db = new mysqli( ... ); } return self::$_db; }

Also set the constructor private, so users won't be able to create useless instances of your class. Or better make it public and throw an exception:

public function __construct() { throw new Exception( 'This class is not supposed to be instantiated' ); }
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

From the PHP documentation: void __construct ([ mixed $args [, $... ]] ). See the void? php.net/manual/en/language.oop5.decon.php
It means the return value is ignored. So even if you return something (another object), it won't modify the type of the object instantiated by new.
3

Define it works fine :)

Try to:

  • compare object hashes returned from both of the methods (does matter when you use object cloning)
  • connect to DB using different credentials (e.g. in unit tests)
  • disconnect from the DB and reconnect again
  • reset the instance of the object (now creating 1000 objects using new fills the memory up)
  • tell some other developer to create in instance of this class, he will for sure look for one::getInstance() method. How do he guesses the behavior of this class?

Singletons are about global state. Looks like here you have global state + some mess.

Improve this answer

Comments

1

Assuming I'm parsing your question correctly, you're asking if it's okay to use the nullity of $db to make sure that you only instantiate one; that's a perfectly valid way to do things, and in fact is what I would recommend. PHP null is explicitly intended to represent a variable with "no value" - a perfect fit for the uninitialized state of the singleton pattern.

Typically, these things will just be called by an intuitive name, e.g. SomeAppDbConn.

Improve this answer

Comments

0 
class one { private static $_selfInstance; public $db; private function __construct() { } public function getDb() { if($this->db == null) $this->db=new mysqli(DB_HOST, DB_USER, DB_PASS, DB_DATABASE); return $this->db; } public static function getInstance() { if( !(self::$_selfInstance instanceof self) ) { self::$_selfInstance= new self(); } return self::$_selfInstance; } }

Access

$db = one::getInstance()->getDb();
Improve this answer

1 Comment

Was there a reason to leave off the second n in $_selfInstace?
0

As Macmade already pointed out, the constructor method doesn't return anything but an instance of the class. Assuming you always want the same instance of mysqli, here's how I'd do it.

class DBInstance { protected static $db; private function __construct() { // intentionally empty } public static function get() { if(self::$db === NULL) { self::$db=new mysqli(DB_HOST, DB_USER, DB_PASS, DB_DATABASE); } return self::$db; } }

It most certainly is a singleton pattern with an added bonus of separating instantiation from the instantiated class.

Improve this answer

3 Comments

A registry would be better, IMHO. Or at least, declares the constructor private, to avoid useless instantiations of the class.
Right you are and I agree with you on using registry. On the other hand, using this method is certainly faster if you're developing a really small app where a database connection is the only fit into this pattern. What do you think?
Sure, and it seems he wants it that way... : )