OOP Best Practices - Queries

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

Post Reply
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

OOP Best Practices - Queries

Post by psurrena »

I have class called generalClass which runs queries, connects to the db and so on. I have another class for a specific section of a website, in this case peopleClass. Right now peopleClass only has queries but may include functions later.
Is it a good or bad idea to keep my queries for this section in the class? How is the overall approach?

Code: Select all

##General Class
class generalClass{
    protected $dbc;
    protected $errorMsg="";
        
    public function __construct(){
        if (stristr($_SERVER['HTTP_HOST'], 'local')||(substr($_SERVER['HTTP_HOST'],0,7)=='192.168')){
            $local=TRUE;
        }else{
            $local=FALSE;
        }
   
        if($local){
            $this->dbc=mysqli_connect('localhost','root','','db') or die (mysql_error());
        }else{
            $this->dbc=mysqli_connect('remote','abc','abc','db') or die (mysql_error());
        }
    }
   
    public function singleResult(){
        $result=mysqli_query($this->dbc,$this->query) or die (mysql_error());
        if($result){
            $row=mysqli_fetch_assoc($result);
        }else{
            $this->errorMsg="Sorry, the result was empty.";
        }       
        return $row;
    }
}
 
### more functions...
 

Code: Select all

##People Class
require_once('./library/class/general.class.php');
 
class peopleClass extends generalClass{
    protected $query;
 
    public function peopleQuery(){
    $this->title=($_GET['title']);
 
    $this->query="SELECT people_fname, people_lname WHERE people_title='this->$title' ORDER BY people_lname ASC";
    
    return $this->query;
}

Code: Select all

##Sample Page
 
#People
require_once './library/class/people.class.php';
 
//Template
$template="default.template.php";
 
//Content
$people=new peopleClass();
$people->peopleQuery();
$item=$people->singleResult();
 
$tags['content'].=$item['people_fname'];
 
User avatar
Eran
DevNet Master
Posts: 3549
Joined: Fri Jan 18, 2008 12:36 am
Location: Israel, ME

Re: OOP Best Practices - Queries

Post by Eran »

What you described here is generally referred to as Models (classes that abstract domain functionality). In web application, most models are indeed database based, and usually extend a base class that defines an API to access the database - as you did with peopleClass extending generalClass.
So definitely, the queries belong in the peopleClass, as they describe a part of the 'people' domain (how to fetch its data).

I have a couple of other suggestion for you though -
- Don't initiate the database connection in the contructor, rather do it on demand (only when a query needs it). A peopleClass object will not always send out a query - so its wasteful to connect to the database every time.
- The generalClass might be better designed as a composed class - acting as a mediator between peopleClass and a separate database class. This way you are not tying yourself to the database specific implementation, but rather to an API (common API for database access is having methods like query(), insert(), update(), delete() etc.)
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: OOP Best Practices - Queries

Post by Christopher »

I think your code looks really screwy. First there are names like "generalClass" which just seems horribly nondiscriptive in every way I can think of. I would reather see something like this:

Code: Select all

class People extends TableDataGateway {
 
    public function findPersonByTitle($title){
        $title = $this->escape($title);
        $sql = "SELECT people_fname, people_lname WHERE people_title='$title' ORDER BY people_lname ASC";
    
        return $this->singleResult($sql);
    }
}
(#10850)
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

Re: OOP Best Practices - Queries

Post by psurrena »

arborint, thanks for cleaning it up a bit, I'm still getting a feel for this.

pytrin, I realize a class doesn't "need" a constructor but should it have one? In theory, is there always something to be done? Also, are you saying the composed class would have insert, update...or is that the db class?
User avatar
Eran
DevNet Master
Posts: 3549
Joined: Fri Jan 18, 2008 12:36 am
Location: Israel, ME

Re: OOP Best Practices - Queries

Post by Eran »

A class should only have a constructor if there is something to be done when its instanced. In this case, creating the database connection every time the class is instanced is wasteful, since you won't necessarily be connecting to the database.

What I was suggesting with the composed class is that it should redirect database calls to a more generic database class. Your generalClass (which I agree with arborint is not the best name) is basically a table class.

Maybe something like:

Code: Select all

 
class DatabaseTable {
    /**
     * Database connection
     * @var Database instance
     */
    protected $_db = null;
    
    /**
     * Table name
     * @var string
     */
    protected $_name;
    
    protected function _connect() {
        if(is_null($this -> _db)) {
            $this -> _db = new Database('Mysql');
        }
        return $this -> _db;
    }
    
    public function fetchAll($sql) {
        return $this -> _connect() -> fetch($sql);
    }
    
    public function insert(array $data) {
        return $this -> _connect() -> insert($data,$this -> _name);
    }
}
 
class People extends DatabaseTable {
    protected $_name = 'people';
    
    public function getPeopleByTitle($title) {
        $sql = "SELECT people_fname, people_lname FROM " 
            . $this -> _name 
            . " WHERE people_title='" 
            . $this -> _connect() -> escape($title)
            . "' ORDER BY people_lname ASC";
        return $this -> fetchAll($sql);
    }
}
 
$people = new People();
$items = $people -> getPeopleByTitle($_GET['title']);
 
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

Re: OOP Best Practices - Queries

Post by psurrena »

This is a bit more abstract than I'm used to so please bear with me.

Does this code:

Code: Select all

$this -> _db = new Database('Mysql');
assume something like:

Code: Select all

class Database{
    public function __construct($database){
        $dbc=mysqli_connect('localhost','root','','$this->database') or die (mysql_error());
        return $dbc;
    }
}
??
User avatar
Eran
DevNet Master
Posts: 3549
Joined: Fri Jan 18, 2008 12:36 am
Location: Israel, ME

Re: OOP Best Practices - Queries

Post by Eran »

Yet, but here the database connection is lazy-loaded. The _connect() delegates database connection to a generic database class on demand.

Code: Select all

 
 protected function _connect() {
         if(is_null($this -> _db)) { //Has a database connection class been created yet?
             $this -> _db = new Database('Mysql'); //Create database object (handles database connections)
         }
         return $this -> _db;
     }
 
and other methods use the database object to query the database:

Code: Select all

 
  public function fetchAll($sql) {
         return $this -> _connect() -> fetch($sql);
     }
    
     public function insert(array $data) {
         return $this -> _connect() -> insert($data,$this -> _name);
     }
 
You could add update() and delete() methods of course;
Post Reply