Problems with singleton

sepan

Can somebody explain to me why the following code does not act "as expected" ?

define("DB_HOST", "localhost");
define("DB_USER", "test");
define("DB_PASS", "test");
define("DB_NAME", "test");

class DBConnection {
  var $handle;      // connection handle

  function DBConnection() {
     print "Creating new database connection\n";
     $this->handle = @mysql_connect(DB_HOST, DB_USER, DB_PASS) or
       die("Could not connect to database");
     mysql_select_db(DB_NAME, $this->handle) or
       die("Could not switch to database ".DB_NAME);
  }

  function &getInstance() {
    static $dbConn;
    print "DBConnection::getInstance\n";
    if (is_null($dbConn))
      $dbConn =& new DBConnection();
    return $dbConn;
  }

  function getLink() {
    return $this->handle;
  }
}

// START HERE

$a = DBConnection::getInstance(); 
$b = DBConnection::getInstance(); 

if ($a === $b) { 
  echo '$a and $b reference the same object.' . "\n"; 
}

--------------------------------------------------------------------------------
When running this script, it prints out the following:
DBConnection::getInstance
Creating new database connection
DBConnection::getInstance
Creating new database connection
$a and $b reference the same object.

Why do I get the "Creating new database connection" message twice? Shouldn't the message be printed only once, because the second time I've already got an instance of DBConnection?
(It seems to run the constructor twice, which is bad, because I create my database connection in here).
I'm running PHP 4.1.2.

onion2k

$a and $b are seperate objects, and being constructed seperately. Hence 2 connections..

sepan

if ($a === $b) {  

  echo '$a and $b reference the same object.' . "\n";  

}

Actually it prints that $a and $b reference the same object.
Don't they?
How would you implement that singleton?

Shrike

Hmm perhaps because $dbConn is static. In the first case you instantiate the class in $dbConn, in the second case you just return it because is_null returns false?

sepan

Originally posted by Shrike
Hmm perhaps because $dbConn is static. In the first case you instantiate the class in $dbConn, in the second case you just return it because is_null returns false?

Well that's the point in using the singleton pattern.
You create only one instance of a class (when calling getInstance the first time). All other calls to getInstance should return the reference to the class.

Shrike

Ah yes, so passing the static variable by reference will mean that it's the same instance in any case, also the point of the Singleton no? Not sure why the constructor is getting run twice, but if you don't pass by reference, it only gets run once.

Weedpacket

Less the & signs, the original code would work as expected in PHP5 (changing a property of $a would see $b change also); however I don't have a version of PHP4 handy right now to confirm if what is needed is

$a = &DBConnection::getInstance();
$b = &DBConnection::getInstance();

Shrike

This seems to work as expected:

<?php
class a {
	function a(){
		echo "making instance<br>";
	}

function make(){
	static $a;
	if(is_null($a)){
		$a = new a;
	}
	return $a;
}
}

$a = a::make();
$b = a::make();
if($a === $b){
	echo "Same object";
}
?>