4
\$\begingroup\$

I recently started working on a project that had been dropped by the previous dev. When looking through the existing code, I came across this singleton Database Class.

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

// Customize when ready
private function __construct(){
    try{
        $this->PDOInstance = new PDO("mysql:database=testdb;unix_socket=/tmp/mysql.sock", "username" , "password"/* , array driver_options*/);
    }catch(PDOException $e){
        throw $e;
    }
}

public static function getInstance(){
    if(!self::$instance){
        try{
            self::$instance = new self();
        }catch(PDOException $e){
            return false;
        }
    }
    return self::$instance;
}

public function __clone(){
    trigger_error("Cloning forbidden on Singleton", E_USER_ERROR);
}

public function __wakeup(){
    trigger_error("Unserializing forbidden on Singleton", E_USER_ERROR);
}

// PDO Function Aliasing
public function __call($function, $args){
    if(method_exists($this->PDOInstance, $function)){
        return call_user_func_array(array($this->PDOInstance, $function), $args);
    }
    trigger_error("Unknown PDO Method Called: $function()\n", E_USER_ERROR);
}
}

I suppose my question revolves around the use of __call() to alias PDO functions. Is this the best way to go about it, and if not, what should be done instead?

As for why the database class doesn't simply extend PDO, from what I understand, PDO has a public __construct(), and when it gets extended, it isn't possible to change the visibility to private, hence this work around.

My instinct is to scrap this class and just rewrite it as a simple PDO wrapper, but I thought I would ask around first.

\$\endgroup\$
2
  • \$\begingroup\$ Singletons are big trouble. I'd rethink your strategy if I were you. gooh.posterous.com/singletons-in-php \$\endgroup\$
    – GordonM
    Feb 8, 2012 at 15:25
  • \$\begingroup\$ @GordonM Thanks for the link. I think I will go ahead and drop it. \$\endgroup\$
    – Grexis
    Feb 9, 2012 at 19:43

1 Answer 1

3
\$\begingroup\$

A simple workaround, assuming that you want to keep the class a Singleton:

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

    public static function getInstance(){
        if(!self::$instance){
            try{
                self::$instance = new self();
            }catch(PDOException $e){
                return false;
            }
        }
        return self::$instance->getPDOInstance();
    }

    public function getPDOInstance() {
        return $this->PDOInstance;
    }
}

Well, more of a factory method now, which is silly. But not as silly as __call() / call_user_func_array. If you absolutely need to keep the class's signature as it is, go for it.

If not, a PDO wrapper would be the better approach.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the review. Looking at a comment on the main question, I'm going to drop the singleton aspect of the class and just write a PDO wrapper. \$\endgroup\$
    – Grexis
    Feb 9, 2012 at 19:44
  • \$\begingroup\$ @Grexis That wasn't really a review, just a workaround :) If you don't need the singleton, go for the wrapper, and when you finish it post it here for a proper review... \$\endgroup\$
    – yannis
    Feb 10, 2012 at 1:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.