Any simpler / better way of making a mysql singlton db class in php?

Question

Here's the one I'm using:

<?php
final class Database {
    private static $oDb;
    public static function init() {
        if(self::$oDb == NULL)
        {
            self::$oDb = mysql_connect('localhost', 'mysql_user', 'mysql_password') or die(mysql_error());
            mysql_select_db('mysql_db_name', self::$oDb) or die (mysql_error());;
        }
        return self::$oDb;
    }
    public function query($sql)
    {
        return mysql_query($sql) or die(mysql_error());
    }
}
?>

Usage:

$oDb = Database::init();
$sql = foo;
$oDb->query($sql);

Assuming that I only want it to connect and execute this one query function, are there any improvements I should make on the class? Memory or efficiency of code?

Also, is there an efficient way I can get the db credentials from a config file? I know I can't use includes inside my class.

Answer 1

That's a simple as it gets, that will work fine.

You can pass your credentials to init();

include(config.php);
$oDb = Database::init( DB_HOST, DB_NAME, DB_USER, DB_PASSWORD );
$sql = foo;
$oDb->query($sql);

Answer 2

You can use an include inside a function inside a class

<?php
final class Database {
    private static $oDb;
    public static function init() {
        if(self::$oDb == NULL)
        {
            include('config.php')
            self::$oDb = mysql_connect(DB_HOST, DB_USER, DB_PASS) or die(mysql_error());
            mysql_select_db(DB_NAME, self::$oDb) or die (mysql_error());;
        }
        return self::$oDb;
    }
    public function query($sql)
    {
        return mysql_query($sql) or die(mysql_error());
    }
}
?>

or you can just pass the variables...

<?php
final class Database {
    private static $oDb;
    public static function init($host, $user, $pass, $name) {
        if(self::$oDb == NULL)
        {
            self::$oDb = mysql_connect($host,$user,$pass) or die(mysql_error());
            mysql_select_db($name, self::$oDb) or die (mysql_error());;
        }
        return self::$oDb;
    }
    public function query($sql)
    {
        return mysql_query($sql) or die(mysql_error());
    }
}
?>

or you can store the credentials in a php.ini file

<?php
final class Database {
    private static $oDb;
    public static function init($db_name) {
        if(self::$oDb == NULL)
        {
            self::$oDb = mysql_connect() or die(mysql_error());
            mysql_select_db($db_name, self::$oDb) or die (mysql_error());;
        }
        return self::$oDb;
    }
    public function query($sql)
    {
        return mysql_query($sql) or die(mysql_error());
    }
}
?>

php.ini file:

mysql.default_host="host"
mysql.default_user="user"
mysql.default_password="password"

Answer 3

I usually use lazy initialization for this sort of situation and only have one public method (in this case), with a private constructor to prevent outside instantiation (per the Singleton pattern):

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

  private function Database() {
    // do init stuff
    require_once('dbconfig.php'); // contains define('DB_USER', 'webuser'); etc...
    $this->conn = mysql_connect(DB_HOST, DB_USER, DB_PASS); // do error checking
  }

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

  public static function query($sql) {
    $instance = self::getInstance();
    return mysql_query($sql, $instance->conn);
  }
}

Then you can just call $dbHandle = Database::getInstance() anytime you need to use it. Or in this case since a static query method is defined, you can use Database::query("select * from xx;"); without having to call any sort of init at all.

Answer 4

For singleton classes, the model that Dan Breen followed is cleanest and very common. However, in this case, I would also allow the getInstance method to accept some parameters so that you can override your default configuration at instantiation time, or just get a reference without creating a connection (both use-cases happen from time to time).

Database.php

require_once("../path/to/config/database.php");

class Database {
  private static $instances = array();

  private function Database($host, $user, $password, $name) {
    // do init stuff
  }

  public static getInstance(
    $host=DB_HOST, $user=DB_USER, $password=DB_PASSWORD, $name=DB_NAME
  ) {
    $key = strtolower($host . $user . $password . $name);

    if ( !$self::instances[$key] ) {
      $self::instances[$key] = new Database($host, $user, $password, $name);
    }
    return $self::instances[$key];
  }
}

..config/database.php:

define("DB_HOST", "localhost");
define("DB_USER", "mrsqlguy");
define("DB_PASS", "!!!");
define("DB_NAME", "just_another_wordpress");

Edit: I've changed it to act more like a flyweight to ensure that you only get one instance for each connect location/database. This addresses your concerns and maintains a degree of flexibility.