0

I have this:

class Database{    
    private $name = '';
    private $user = '';
    private $password = '';   
    private $host = 'localhost';
    private $prefix = '';   
    private $connection_handle = null;

    private function Connect(){
        $this->connection_handle = mysql_connect($this->host, $this->user, $this->password); 

        if( !$this->connection_handle ){
            die( 'Could not connect: ' . mysql_error() );
        }else{
            mysql_select_db( $this->name, $this->connection_handle );
        }
    }

    private function Close(){
        mysql_close( $this->connection_handle );
    } 

    public static function Query( $query ){
        $this->Connect();   
            $result = mysql_query( $query, $this->connection_handle );
            if( !$result ){
                die( 'Error: ' . mysql_error() );
            }else{
                $DatabaseQuery = new DatabaseQuery();
                    $DatabaseQuery->result = $result;       
                    $DatabaseQuery->mysql_num_rows = mysql_num_rows($result);

                return $DatabaseQuery;
            }
        $this->Close();
    } 
}

I have made Query static because it will be called like "Database::Query" The other functions need not be accessed by any other class so I made them private...

I get this error when accessing the Connect function from the Query function.

"Using $this when not in object context"

Having thought private makes those variables private to this class but available to all methods within it, I am having issues with getting scope of the functions...

I can do self::Connect() but I don't really understand it enough to warrant the use of it...

Can you explain the difference between self:: and %this->

I also cannot get access to my private variables and they really never need to be accessed outside of this class, once again I made them private because of this... and yet $this->connection_handle has no scope to the variable.

Do I really need to make them all public? there must be a way to make them available for this class only and extensions of it?

----------------------EDIT:///

I have ended up making all my variables private static. Is this an acceptable way of doing this?

----------------------EDIT:///

I have now got this code:

class Database{    
    private static $name = '';
    private static $user = '';
    private static $password = '';   
    private static $host = 'localhost';
    private static $prefix = '';   
    private static $connection_handle = null;

    private static function Connect(){
        self::$connection_handle = mysql_connect(self::$host, self::$user, self::$password); 

        if( !self::$connection_handle ){
            die( 'Could not connect: ' . mysql_error() );
        }else{
            mysql_select_db( self::$name, self::$connection_handle );
        }
    }

    private static function Close(){
        mysql_close( self::$connection_handle );
    } 

    public static function Query( $query ){
        self::Connect();   
            $result = mysql_query( $query, self::$connection_handle );
            if( !$result ){
                die( 'Error: ' . mysql_error() );
            }else{
                $DatabaseQuery = new DatabaseQuery();
                    $DatabaseQuery->$result = $result;       
                    $DatabaseQuery->$mysql_num_rows = mysql_num_rows($result);

                return &$DatabaseQuery;
            }
        self::Close();
    } 
}

But returning the pointer reference is not working? I'm actually not so up to date with PHP pointers, anyone shed some light on what I'm doing wrong here?

It's because i'm declaring the new instance of the DatabaseQuery class within the function isn't it. HELPPP :)

EDIT::///////

I have finished my class and it looks like this:

class DatabaseQuery{
    public $result;
    public $mysql_num_rows;        
}

class Database{    
    private static $name = '';
    private static $user = '';
    private static $password = '';   
    private static $host = 'localhost';
    private static $prefix = '';   
    private static $connection_handle = null;

    protected function Connect(){
        self::$connection_handle = mysql_connect(self::$host, self::$user, self::$password); 

        if( !self::$connection_handle ){
            die( 'Could not connect: ' . mysql_error() );
        }else{
            mysql_select_db( self::$name, self::$connection_handle );
        }
    }

    protected function Close(){
        mysql_close( self::$connection_handle );
    } 

    public static function FetchQueries( &$queries ){
        $db_query = array();
            self::Connect();   
                foreach( $queries as $key => $query ){
                    $result = mysql_query( $query, self::$connection_handle );
                    if( !$result ){
                        die( 'Error: ' . mysql_error() );
                    }else{
                        $DatabaseQuery = new DatabaseQuery();
                            $DatabaseQuery->result = $result;       
                            $DatabaseQuery->mysql_num_rows = mysql_num_rows($result);  
                            $db_query[ $key ] = $DatabaseQuery; 
                    }
                }
            self::Close();
        return $db_query;
    } 
}

Now you can call it in an Model View Control (MVC) way because you are defining all data at the top (or in a separate file)

$queries = array(
    "bottoms" => "SELECT * FROM bottoms", 
    "users" => "SELECT * FROM users"           
);            
$dbr = Database::FetchQueries( $queries );


//display some users
while( $row = mysql_fetch_object( $dbr["apps"]->result ) ){
    echo $row->title; 
}    

//display some bottoms
while( $row = mysql_fetch_object( $dbr["bottoms"]->result ) ){
    echo $row->title; 
}

Can you tell me what you think of this MVC method?

hakre
  • 193,403
  • 52
  • 435
  • 836
Jimmyt1988
  • 20,466
  • 41
  • 133
  • 233
  • For some more flexible code and examples, please see: http://stackoverflow.com/a/11580420/367456 - also consider to use PDO or Mysqli directly, they ship with pre-made objects already you can spare the whole code then. – hakre Dec 13 '12 at 02:35

1 Answers1

2

You should make your Database class instantiable. Right now, you are opening a new connection for each query and this has an abysmal performance (really bad). You can make this class a singleton (in fact that is usually how it's done because normally it makes sense to have only one connection to the DB).

So your code (very simplified) should be like:

class Database
{
  private static $instance;

  protected function __construct() {
    self::$instance = mysql_connect(...);
    // do some error checking!
  }

  protected function __clone() {}

  public static function getInstance() {
    if (!isset(self::$instance)) {
      self::$instance = new Database();
    }
    return self::$instance;
  }

  public function query($sql) {
    do mysql_query(...) and error checking, return $result or your own DatabaseQuery()
  }
}

This way you can't directly make queries calling a static function, but you could add it with another function inside this class:

public static function makeQuery($sql) {
  $db = self::getInstance();
  return $db->query($sql);
}

To use this class with an instance, you should call $db = Database::getInstance();, and $db is a Database object which you can call it's public methods normally. If you just want to continue making static calls to make a query, just call $result = Database::makeQuery($sql). Either way, it's guaranteed that you only have one connection to the database (that's what the singleton does) so you don't have the current overhead of initiating a new connection for each query.

ALSO consider using mysqli or pdo-mysql instead of the old and deprecated mysql_* functions (see the warning at the top of mysql_connect). For more info and benefits of each API, see Choosing a MySQL API

Carlos Campderrós
  • 22,354
  • 11
  • 51
  • 57
  • is protected the same as private static? – Jimmyt1988 Dec 12 '12 at 12:49
  • I was going to make $query an array of queries that can be performed at the beginning of the page so to try and maintain an MVC kind of layout. – Jimmyt1988 Dec 12 '12 at 12:53
  • I have added a new edit to my post, i'd really appreciate you taking a look and seeing what you think of it? Can you see any issues I may come across? – Jimmyt1988 Dec 12 '12 at 13:32
  • @JamesT protected is a mix between `public` and `private`. You can access `protected` properties/methods from child classes, but not from the outside. It has nothing to do with `static`, but you can have a `protected static` property/method if you want. – Carlos Campderrós Dec 12 '12 at 13:38
  • @JamesT your new edit should work, but I really don't think it's a good design. If you extend your framework you'll likely see that you need to perform queries in other parts of your application (think of using the Observer pattern or a plugin system: you'll want to do queries in the plugins handlers) – Carlos Campderrós Dec 12 '12 at 13:46
  • Thanks for your input there Carlos. Really appreciated. – Jimmyt1988 Dec 12 '12 at 13:51