ElkArte Community

At the moment we have:

	private static $_db = null;
[...]
	/**
	 * Returns a reference to the existing instance
	 */
	static function db()
	{
		return self::$_db;
	}

Then:

/**
 * Retrieve existing instance of the active database class.
 *
 * @return Database
 */
function database()
{
	global $db_type;

	// quick 'n dirty retrieval
	if ($db_type == 'mysql')
		$db = Database_MySQL::db();
	elseif ($db_type == 'postgresql')
		$db = Database_PostgreSQL::db();
	elseif ($db_type == 'sqlite')
		$db = Database_SQLite::db();

	return $db;
}

In other words, a singleton class called from a function.

I was wondering, why not have a "proper" class and a function with a static variable?
In other words, remove the singleton part from the class and change to:

/**
 * Retrieve existing instance of the active database class.
 *
 * @return Database
 */
function database()
{
	global $db_type;
	static $db;

	if (isset($db))
		return $db;

	// quick 'n dirty retrieval
	if ($db_type == 'mysql')
		$db = new Database_MySQL;
	elseif ($db_type == 'postgresql')
		$db = new Database_PostgreSQL;
	elseif ($db_type == 'sqlite')
		$db = new Database_SQLite;

	$db->initiate();

	return $db;
}

Or something like that.

Any php or software design expert around? O:-)

Okay, just to make it more clear, this is what I meant:
https://github.com/emanuele45/Dialogo/commit/1f477e5941b0762e130e3df0b4280984fe5afde7

makes sense (and no, I'm not a php expert).

No expert here either ... but to me it seems less obvious that the class is a singleton if done that way .. I'm not sure I like the way that static $db and $tbl are defined.  To me it just seems more tidy if the class takes care of itself.

Today it just does this (well with that function call in there since we have to call the right class) and its not called instance but I put that there so it was more clear to me.

class DataBase
{
	private static $instance = null;

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

$db = DataBase::getInstance();

Yep, because in fact it is not a singleton.

The main reason I was thinking at this is because of the fact that almost any method of the class has a parameter you can use to override the connection (except it works only for mysql), but in that case you have to initiate the new connection all by yourself, because otherwise, using initiate, the class is "re-initialized" completely and all the old connection data are wiped out.

Instead, with this method, you should be able to re-instantiate the class with another connection without touching the current one...I think.