Abstract database class feedback

[franko75]

Hi, I've been playing around with an old (simple) database class i'd written a while back and I want to improve my understanding of OOP, so i'm trying to improve the design to make it as reusable as possible. I know PDO is available as an abstraction layer, but this is mainly for learning purposes so i'd be interested in any feedback.

I decided to use an abstract class, instead of an interface, as there is a level of implementation i can add to the constructor of the abstract class and which can apply to any extending classes (mysql, postgres etc).

This can obviously be fleshed out to add a lot more flexibility and functionality but at the moment i want to make sure i have the general design right and am not misunderstanding any of the concepts.

abstract class Database {
    
    protected $config;
    
    public function __construct(Config $conf) {
        
        $this->set_config($conf)
             ->connect()
             ->select_db();
    }
    
    protected function set_config($conf)
    {
        $this->config = $conf;
        return $this;
    }
    abstract protected function connect();
    abstract protected function select_db();
    abstract public function query($query);
    abstract public function fetch_assoc($queryResource);
    abstract protected function disconnect();
    abstract protected function error_code();
} 

mysql.php

class Mysql extends Database {
    
    private $db;
    private $link;

    protected function connect()
    {    
        $this->link = mysql_connect($this->config->hostname, $this->config->username, $this->config->password);
        
        if ( ! $this->link)
        {
            return $this->error_code();
        }
        return $this;    
    }

    protected function select_db()
    {
        $this->db = mysql_select_db($this->config->database);
        if ( ! $this->db)     
        {
            return $this->error_code();
        }
    }
    
    public function query($qry) {
        
        return mysql_query($qry);
    }
    
    public function fetch_assoc($res) {
        
        return mysql_fetch_assoc($res);
    }
    
    protected function error_code() {
        
        return mysql_error($this->link); 
    }
    
    protected function disconnect() { 
        
        is_resource($this->link) and mysql_close($this->link);
    }
} 

I'm also wondering if there's a neater way to handle any errors from the query or fetch_assoc methods, other than an if/else statement - i.e in my return statement can i do something like this (even though i know this particular statement doesn't work):

return mysql_error($this->link) OR $this->error_code(); 

[AnthonySterling]

You could throw an Exception, then catch / re-throw where appropriate.

<?php
class DatabaseException extends Exception
{
}

class MySQL extends Database implements IDatabase
{
    public function query($sSQL)
    {
        if(error)
        {
            throw new DatabaseException('Error Executing Query:- ' . $sSQL);
        }
    }
}

try
{
    $oDatabase = new MySQL();
}
catch(DatabaseException $oException)
{
    //Log error, exit or even re-throw for a different part of the application to catch.
}
?>

[franko75]

Thanks SilverBullet - i've never been quite sure where to use exception handling code, whether i should be adding try/catch blocks inside methods or not. Using your example, am i right in thinking i can throw exceptions from the methods and catch them in the calling code where i'm attempting to instantiate an object, use its methods etc?

Also, would you recommend defining an interface as well an abstract class on this occasion? I see you have class MySQL extends Database implements IDatabase.

Cheers for the help - trying to get my head round things as best i can!

[oddz]

You could do something like the below. Then always program to the DBConnectable interface. The advantage of this would be a common interface between all connection types. So using a MysqlI connection in place of a PDO connection would be as simple as changing one line of code because instances of DBConnectable are interchangeable as long as you program to that interface.

interface DbConnectable  {

    public function connect();
    public function select_db();
    public function query($sql);
    public function fetch_assoc();
    public function disconnect();
    public function error_code();

}

abstract class DbConnection implements DbConnectable {

    public function connect() {
    }    
    public function select_db() {
    }
    public function query($sql) {
    }
    public function fetch_assoc() {
    }
    public function disconnect() {
    }
    public function error_code() {
    }

}

class PDOConnection extends DbConnection {

    public function query($sql) {
    
    }

}

class MySQLIConnection extends DbConnection {

    public function error_code() {
    
    }

} 

[franko75]

Thanks oddz - i think programming to an interface is a really good solution for this situation.

I see that I have set up my abstract class differently from you, in that i have some functionality in my constructor which should be common to all extending classes. I thought this would be the right thing to do here, to save repeating the code in all extending classes' controllers. Also, i made the majority of methods abstract in my abstract controller, whereas i see you have not done this. The reason i went for abstract methods is that these methods will require different implementation by each extending class. What would you recommend here? Am i on the right lines with this approach?

[oddz]

The reason i went for abstract methods is that these methods will require different implementation by each extending class. What would you recommend here?

Each interface method can be overridden for class specific implementation. However, tasks that may be same for each connection need only to be defined inside the base abstract class.

The interface is used so that another class can easily be defined by only implementing that interface. It isn't required that the concrete classes extend DbConnection. Its only required that they implement DBConnectable. DbConnection is only there to define similar implementations while avoiding repetition. This is a simple Template pattern.

The only way that the application should communicate with a DbConnectable instance is through the provided interface methods.

The important thing to understand is that the implementation of each method doesn't matter. Communication will always be the same which yields all objects interchangeable so long as the DbConnectable interface is programmed to and the object is a DbConnectable instance (implements DbConnectable).

This makes it simple to create a new connection which is interchangeable with all other DbConnectable classes.

class MyWeirdDbConection implements DbConnectable {

    public function connect() {
    }    
    public function select_db() {
    }
    public function query($sql) {
    }
    public function fetch_assoc() {
    }
    public function disconnect() {
    }
    public function error_code() {
    }

} 

[franko75]

Great oddz - makes perfect sense now, thanks again for your help.

[franko75]

Ok, i've now hit a problem with the above structure - I want to use the singleton pattern to prevent multiple instantiations of db connections. In my abstract class, I have the following concrete methods:

    public function __construct($conf) {
        
        $this->set_config($conf)
             ->connect()
             ->select_db();
    }
    
    
   public function set_config($conf)
    {
        $this->config = $conf;
        return $this;
    } 

Then, in my concrete, child classes - e.g Mysql, I add the appropriate functionality to the connect() and select_db() methods. However, if I want to use the singleton pattern, concrete classes such as Mysql will not be able to inherit the constructor method from the abstract class, given that the constructor in a singleton must be private.

Would my best option here be to remove the constructor and set_config methods from the abstract class and simply handle all of this functionality in each individual concrete class? My concern with this would be that there would be a fair amount of repetition in each child class.

[AnthonySterling]

What about providing a constructor in the child class to override the parent constructor?

[oddz]

Only way to do this involves overriding the static methods in base class. Without support for late static binding that is the only way.

<?php
abstract class Db {

    protected static $db;
    
    protected function __construct($conf) {     

        $this->set_config($conf)
        ->connect()
        ->select_db();
    
    }
    
   public function set_config($conf) {

        $this->config = $conf;
        return $this;

    } 

    
    abstract public static function getInstance($conf);

}

class ImprovedDb extends Db {

    public static function getInstance($conf) {

        if(is_null(self::$db)) {
            $class = __CLASS__;
            self::$db = new $class($conf);
        }
        return self::$db;    
    
    }

}

class PDODb extends Db {

    public static function getInstance($conf) {

        if(is_null(self::$db)) {
            $class = __CLASS__;
            self::$db = new $class($conf);
        }
        return self::$db;    
    
    }

}

class BasicDb extends Db {

    public static function getInstance($conf) {

        if(is_null(self::$db)) {
            $class = __CLASS__;
            self::$db = new $class($conf);
        }
        return self::$db;    
    
    }

}
?>

[franko75]

Thanks again oddz (and SilverB) - have this up and running now, plus I think i actually understand what is going on - which is a bonus!