4
\$\begingroup\$

I have a situation where I have to connect to multiple schema to populate a single page.

I altered our singleton db class in to help me do this.

It will now destroy and re-declare itself whenever we pass a new set of db credential.

Will the PHP pros mind code-reviewing this? we do not have code reviews here and this is a pretty big change as it will affect just about everything in our system... so I'm hoping maybe someone here can lend me a second pair of eyes...

I'm a bit iffy on the destroy() method, maybe there's a better to do this?

<?PHP
Class DB{
private static $instance;   //instance of db.
private $cursor;            //connection cursor
private $db_user;           //db username
private $db_pass;           //db password
private $db_sid;            //schema id
/**
* not allowed to publicly create this. Should be called ONLY from self::get_instance.
* Should be the ONLY place where db cursor is set.
*
* @see self::get_instance($db_user, $db_pass, $db_sid);
*/

private function __construct($db_user, $db_pass, $db_sid){
    $this->db_user = $db_user ? $db_user : DATABASE_USER;
    $this->db_pass = $db_pass ? $db_pass : DATABASE_PASS;
    $this->db_sid = $db_sid ? $db_sid : DATABASE_SID;
    $this->get_cursor();
}

/*
* not allowed to publicly clone this.
*/
public final function __clone(){}


/*
* retrieves the db connection.
*/
private function get_cursor(){
    if(isset($this->cursor)){
        return true;
    }

    $this->cursor = oci_pconnect($this->db_user, $this->db_pass, $this->db_sid);
    if($this->cursor == false){
        throw new exception__DBException();
        return false;
    }
    return true;
}


/**
* THE ONLY PUBLIC METHOD FOR GRABBING DB INSTANCE.
* Will automatically self destruct and reconnect with a new connection if new db_user/db_pass@db_sid is passed.
*
* @param string $db_user
* @param string $db_pass
* @param string $db_sid
* @return db the DB instance.
*
*/
public static function get_instance($db_user, $db_pass, $db_sid){

    /*
    We check if the instance is declared. If not declared we simply create a new DB instance and return.
    */
    if(!(self::$instance instanceof DB)){
        self::$instance = new DB($db_user, $db_pass, $db_sid);
        return self::$instance;
    }


    /*
    If we're here, there must be a db instance already declared. We will check if that instance have the same user/pass@sid as the function params.
    If not, it means we want to connect to a different DB. We destroy previous connection and reset instance again with new credential.
    Else we proceed as normal (return instance w/o doing anything).
    */

    if((self::$instance->get_user() !== $db_user) ||
        (self::$instance->get_pass() !== $db_pass) ||
        (self::$instance->get_sid() !== $db_sid)){

        self::$instance->destroy();
        self::$instance = new DB($db_user, $db_pass, $db_sid);
    }

    return self::$instance;
}

//unsets cursor, frees resource.
private function destroy(){
    if(isset($this->cursor)){
        oci_close($this->cursor);
        unset($this->cursor);
    }
}

//getters
public function get_user(){
    return $this->db_user;
}

public function get_pass(){
    return $this->db_pass;
}

public function get_sid(){
    return $this->db_sid;
}

/**
* executes the sql statement.
* @param string $statement
* @return the result.
*
*/

public function execute_query($statement){
    $rows_returned = 0;
    $result_set = array();

    $stmt = oci_parse($this->cursor, $statement);
    ocisetprefetch($stmt, 10000);
    if(oci_execute($stmt)){
        while($result = oci_fetch_array($stmt, OCI_ASSOC + OCI_RETURN_NULLS)){
            $rows_returned += 1;
            $result = array_change_key_case($result, CASE_LOWER);
            $result_set[] = $result;
        }
    }else{
        throw new exception__DBException(...);
    }
    return $result_set;
}

    /**
* executes the sql statement with binding vars.
* @param string $statement SQL statement
    * @param array $variables looks like array[binding_key]=variable.
* @return the result.
*/

public function execute_query_by_bind($statement, $variables){
    $rows_returned = 0;
    $result_set = array();
    $stmt = oci_parse($this->cursor, $statement);

    if(is_array($variables)){
        foreach($variables as $key => $value){
            oci_bind_by_name($stmt, ':'.$key, $variables[$key]);
        }
    }

    ocisetprefetch($stmt, 10000);

    if (oci_execute($stmt)){
        while($result = oci_fetch_array($stmt, OCI_ASSOC + OCI_RETURN_NULLS)){
            $rows_returned += 1;
            $result = array_change_key_case($result, CASE_LOWER);
            $result_set[] = $result;
        }
        oci_free_statement($stmt);
    }else{
        throw new exception__DBException();
    }
    return $result_set;
}
}
?>
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I'm not sure if I necessarily agree with the idea of destroying the singleton and recreating the connection with different credentials. Perhaps there's a specific use-case for this that you have in mind, but I would personally not want to do this.

I once solved an issue that I think is similar to yours. The application had a singleton that controlled access to the database, in a similar manner to your application right now. We received a new requirement that involved us making a new connection to an accounting database on another host. I simply added a new getter method called getAccountsConnection(), which would return a database instance that behaved in a similar fashion with different credentials.

Now, I'm not sure if you'd necessarily call this a "singleton" any more, but I felt it was a very clean solution to the issue, and as far as I know, it's still in use today. The important thing to note here is that the two connections would exist at the same time when they were called, and the behaviour of our database layer was to not actually connect until the connection was required.

\$\endgroup\$
2
  • \$\begingroup\$ Cool, thanks for the reply, I thought about doing this as well, but it's so rare that this happens that I thought I would just keep it as one connection. Also, this class gets called as a part of a report generator that loops over hundreds of schemas and generates hundreds of separate reports sequentially. I organized them so that no two schemas are ever needed at the same time though, to make destroying and reconnecting more efficient, which is part of the reason I kept this as a constant single connection. \$\endgroup\$
    – jack97
    Jul 19, 2011 at 21:42
  • \$\begingroup\$ @Steve could you show an example about the getAccountsConnection() ? \$\endgroup\$ Mar 24, 2014 at 12:42

Your Answer

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

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