2

Consider the following code below, I've been thought by Lynda.com to create a database class like this, my question is why not create a static method for the database entirely instead of storing an instance into a static property?

<?php
class Database {

  private $conn;
  private static $init;

  public function __construct() {
     $this->conn = new mysqli('localhost','root','root','mydb');
  }

  public static function getInstance() {
     self::$init = new self();
     return self::$init;
  }

}

$db = Database::getInstance();

?>
Vidhya Kumar
  • 128
  • 1
  • 7
  • 2
    Both are bad anyway, because they create global state and implicit dependencies, so don't do it. – ChocoDeveloper Nov 25 '12 at 06:57
  • 2
    That code is missing a `static` specifier on `getInstance`. This is called a [singleton pattern](http://stackoverflow.com/questions/4595964/who-needs-singletons). – DCoder Nov 25 '12 at 06:57
  • Are you *sure* this is how your teacher said to do it? I smell important details missing. – goat Nov 25 '12 at 07:05
  • You should first ask: *"Why not just create an object instance of the `Mysqli` database class and pass it around?"* A new inside a constructor just to assign it to a variable for static access later? Makes not much sense. – hakre Nov 25 '12 at 12:30
  • Do not use `?>` (remove it) unless you know you need it. – Marcin Orlowski Nov 25 '12 at 12:32
  • Well technically my lecturer is none other than Lynda.com so yeah the lecturer explained that creating a single instance connection to the database and storing in a static property for later use reduces overhead and improves performance rather than creating multiple connection instances. – Vidhya Kumar Nov 25 '12 at 13:44
  • Yeah @DCoder I forgot the static specifier for the method, my apologizes. – Vidhya Kumar Nov 25 '12 at 13:48
  • The problem is, your code instantiates a new mysqli instance *every time* you call the `Database::getInstance()` method. If this is supposed to be a singleton, then, you should instantiate a new mysql only if `self::$init` is null. – goat Nov 25 '12 at 18:38

3 Answers3

2

If you want to use singleton you should to protect __construct()

class DB
{
  private static $instance;

  private function __construct($args)
  {
    // do some
  }

  public static function getInstance()
  {
    // get instance
  }
}

$query = 'SELECT etc...';
$stmt = DB::getInstance()->prepare($query);

I use this pattern in DB class. But if you have more than 1 connection you should NOT! use singleton.

Nikita Melnikov
  • 209
  • 1
  • 8
1

My guess is that code you posted was intended to be the following because it looks like it was intended to be a singleton. I've only changed the getInstance() method.

class Database {

  private $conn;
  private static $init;

  public function __construct() {
     $this->conn = new mysqli('localhost','root','root','mydb');
  }

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

}

$db = Database::getInstance();

I think this should clear up the confusion of why a static instance variable is used.

If this wasn't intended to be a singleton, then your question of "why didn't they just use a static method" should have the answer "they should have".

goat
  • 29,650
  • 7
  • 65
  • 92
0

Just to clarify, $db is a Database object, which is instantiated by calling static method getInstance().

why not create a static method for the database entirely?

A static method is defined inside a class, however it can be called without having to instantiate the class. In other words, a static method belongs to the class, since in can be called without instantiating an object.

You can add methods to the Database class to interact with the database as you deem necessary.

mbinette
  • 4,884
  • 3
  • 22
  • 31
  • When I mean static method entirely, why not create a database connection in a static method instead of a constructor. – Vidhya Kumar Nov 25 '12 at 13:51