1

I am new to PHP OOP. Below is my VERY FIRST class file. I would like to add more flexibility to this code, add functions so I can run queries and even assign the results (either fetch_assoc/fetch_array) to an array (Or var, etc) for latter use.


The problem I am having with running queries, is that I am not able to put together both classes (nesting?): $db->Query->Select('myTable'); OR $db->Query->Select('myTable')->Where($pageID); OR $db->Query->Select('myTable')->Where($pageID)->OrderBy('name');


ALSO, I would appreciate if you let me know if there's something I am doing wrong in this code AND suggestions for improvements, so I can write better php classes in the future =).

<?php
require( $_SERVER['DOCUMENT_ROOT'] . '/database/database-connection-info.php');
 //This file (database-connection-info.php) contains $config[];

class db {
    private $db;
    private $type = 'read-only';
    //$config['db']['dbh']          = Database Host
    //$config['db']['dbn']          = Database Name
    //$config['db'][$type]['dbu']   = [User type][user name]
    //$config['db'][$type]['dbp']   = [User type][password]


    public function __construct($type = null) {
        global $config;
        $this->config = $config;
        $this->Connect($type);
    }


    public function Connect($type) {
    global $config;
        switch( $type ) {
        case 'admin':
        $this->type = 'admin';
        $this->db = mysql_connect( $this->config['db']['dbh'] , $this->config['db'][$type]['dbu'] , $this->config['db'][$type]['dbp'] );
        return $this->db;

        default:
        $this->type = 'read-only';
        $type = 'read';
        $this->db = mysql_connect( $this->config['db']['dbh'] , $this->config['db'][$type]['dbu'] , $this->config['db'][$type]['dbp'] );
        return $this->db;
        }
    }


    public function Close() {
        mysql_close();
        mysql_close($this->db);
        $this->type = 'Closed';
    }


    public function Type() {
        return "<b>Database connection type</b>: " . $this->type . "<br/>";
    }


    public function Status() {
        if( !@mysql_stat($this->db) )
             { return "<b>Database connection status</b>: There is no database connection open at this time.<br/>"; }
        else { return "<b>Database connection status</b>: " . mysql_stat($this->db) . "<br/>"; }
    }
}

//For testing purposes
$db = new db(admin);
echo $db->Type();
echo $db->Status();
echo $db->Close();
echo $db->Status();
echo $db->Type();
?>
PeeHaa
  • 71,436
  • 58
  • 190
  • 262
Omar
  • 11,783
  • 21
  • 84
  • 114

1 Answers1

3

Two significant improvements:

  • Don't use a global for the config, instead pass the config in the constructor. This allows you to instantiate db objects to other databases without having to hack a global.

  • Stop using the native [depreacted] mysql_* library and switch to a modern API such as PDO or MySQLi. This is a good PDO tutorial, especially in your case because you are coming from the native mysql_* perspective.

I also noticed this:

$db = new db(admin);

PHP will look for a constant admin here, it will fail to find it and assume the string admin was intended. If you ever had a constant defined as admin then you would observe strange behavior there (or what you might think is string anyway). Change to a string:

$db = new db('admin');
MrCode
  • 63,975
  • 10
  • 90
  • 112
  • How do I pass the `$config[]` to the constructor? I tried `public $varname = $config[];` and it gave me an error. – Omar Nov 14 '12 at 10:56
  • Declare in the constructor `__construct($type = null, $config)`, remove the global from constructor, then when instantiating: `$db = new db('admin', $config);` – MrCode Nov 14 '12 at 11:04