Page 1 of 2

My First PHP Class

Posted: Fri Jul 18, 2008 10:56 am
by bob_b
~pickle | Please use [ code=html ], [ code=php ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.


I'm having a go at moving to OO PHP and here is my first class. It seems to work but I was wondering what others thought. It's designed to handle connections, queries and disconnections to a MySQL database. I'm using a custom error handler for failures. How can I improve it?

Code: Select all

<?php
 
/**
 * DatabaseHandler.php
 * Class for database operations.
 * Sevices include: database connection, disconnection, database queries and free result calls.
 */
 
class DatabaseHandler {
 
  /*
   * Declare some variables
   */
  private $site;
  private $dbHost;
  private $dbUser;
  private $dbPass;
  private $dbName;
  private $connection;
  private $dbSelected;
  private $query;
  private $free;
  private $disconnect;
  
  /*
   * Makes a connection to the database server.
   * Then selects the database.
   * Database connection parameters are located in db_config.php.
   * Returns the database connection handle on success.
   */  
  public function db_connect($site) {
    
    require($site . "/db_config.php");
    $connection = @mysql_connect($dbHost, $dbUser, $dbPass);
    if(!$connection) {
      trigger_error("Could not connect to database server.", E_USER_ERROR);
    } else {
       $dbSelected = @mysql_select_db($dbName, $connection);
       if(!$dbSelected) {
         trigger_error("Could not select database - " . $dbName, E_USER_ERROR);
       } else {
         return $connection;
       }
    }
  }
  
  /*
   * Issues queries to the database.
   * Returns the result set on success.
   */ 
  public function query($query) {
    $result = mysql_query($query);
      if(!$result) {
        trigger_error("Could not issue query.", E_USER_ERROR);
      } else {
        return $result;
      }
  }
  
  /*
   * Free result.
   * Returns true on success.
   */
  public function free_result($connection) {
    $free = @mysql_free_result($connection);
    if(!$free) {
      trigger_error("Could not free result set.", E_USER_ERROR);
      } else {
        return $free;
      }
  }
 
  /*
   * Disconnects from the database server.
   * Returns true on success.
   */
  public function db_disconnect($connection) {
    $disconnect = @mysql_close($connection);
    if(!$disconnect) {
      trigger_error("Could not close database connection.", E_USER_ERROR);
    } else {
      return $disconnect;
    }
  }
  
}  // end class
 
?>
And here is how I use it to connect:

Code: Select all

<?php
 
include("includes.php");
 
$dbConn = new DatabaseHandler;
 
// $site is defined in includes.php
$connection = $dbConn->db_connect($site);
 
$query = "SELECT * FROM test";
 
$result = $dbConn->query($query);
 
while($row = @mysql_fetch_row($result)) {
  $data = $row[0];
  echo "<p><strong>" . $data . "</strong></p>";
}
 
$free_result = $dbConn->free_result($result);
 
$disconn = $dbConn->db_disconnect($connection);
 
?>
Many thanks in advance for any feedback or help!


~pickle | Please use [ code=html ], [ code=php ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.

Re: My First PHP Class

Posted: Mon Jul 21, 2008 6:06 am
by bob_b
I've made some improvements:

Code: Select all

<?php
 
/**
 * DatabaseHandler.php
 * Class for database operations. Services avialable:
 * database connection, disconnection, queries, free result, num_rows, affected_rows, last insert id.
 */
 
class DatabaseHandler {
 
  /*
   * Declare some private variables.
   */
  private $site;
  private $dbHost;
  private $dbUser;
  private $dbPass;
  private $dbName;
  private $connection;
  private $dbSelect;
  private $query;
  private $free;
  private $disconnect;
  
  /*
   * Makes a connection to the database server and attempts to select the database.
   * Database connection parameters are located in db_config.php.
   * Triggers an error on failure. Returns the database connection handle on success.
   */  
  public function db_connect($site) {
    
    /*
     * Include the database connection parameters.
     */
    require($site . "/db_config.php");
    
    /*
     * Attempt to make a connection. Trigger an error on failure.
     * If successful attempt to select the database. Trigger an error on failure.
     * Return the connection link identifier if no errors are detected.
     */
    $this->connection = @mysql_connect($dbHost, $dbUser, $dbPass);
    if(!$this->connection) {
      trigger_error("Could not connect to database server.", E_USER_ERROR);
    } else {
       $dbSelect = @mysql_select_db($dbName, $this->connection);
       if(!$dbSelect) {
         trigger_error("Could not select database.", E_USER_ERROR);
       } else {
         return $this->connection;
       }
    }
  }
  
  /*
   * Issues user defined queries to the database.
   * Triggers an error on failure. Returns the result set on success.
   */ 
  public function query($query) {
    $this->result = @mysql_query($query);
      if(!$this->result) {
        trigger_error("Could not issue query.", E_USER_ERROR);
      } else {
        return $this->result;
      }
  }
  
  /*
   * Returns the number of affected rows.
   */
  public function affected_rows() {
    $affectedRows = @mysql_affected_rows();
    return $affectedRows;
  }
  
  /*
   * Returns the number of selected rows.
   */
  public function num_rows() {
    $numRows = @mysql_num_rows($this->result);
    return $numRows;
  }
  
  /* Returns the last insert id.
  */
  public function insert_id() {
    $insertID = @mysql_insert_id();
    return $insertID;
  }
  
  /*
   * Attempts to free result memory.
   * Triggers an error on failure. Returns true on success.
   */
  public function free_result() {
    $free = @mysql_free_result($this->result);
    if(!$free) {
      trigger_error("Could not free result set.", E_USER_ERROR);
      } else {
        return $free;
      }
  }
 
  /*
   * Attempts to disconnect from the database server.
   * Triggers an error on failure. Returns true on success.
   */
  public function db_disconnect() {
    $disconnect = @mysql_close($this->connection);
    if(!$disconnect) {
      trigger_error("Could not close database connection.", E_USER_ERROR);
    } else {
      return $disconnect;
    }
  }
  
}  // end class
 
?>
It seems to work nicely. Hopefully, I'm heading in the right direction ...

Re: My First PHP Class

Posted: Mon Jul 21, 2008 10:59 am
by bob_b
~pickle | Please use [ code=html ], [ code=php ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.
Oh, well my post looks exactly the same. No idea what I did wrong there. Shame this was all the feedback I got!

Re: My First PHP Class

Posted: Mon Jul 21, 2008 11:10 am
by pickle
  • I'd modify it to use the mysqli library rather than the old mysql library. It lets you (or at least it seems like it to me) do things more efficiently.
  • The private variables you have for storing the database credentials are superfluous - you don't use them anywhere. That's for the best anyway as there's no need to store them beyond your initial connection function.
  • I can see why you made the db_connect() function require a $site variable, but you're kind of limiting yourself there. What if, in the future, you want to put your database credentials in a different file? I recommend 2 things:
    1. rename your db_connect() function to __construct() so it will be run automatically when the object is instantiated. You'll never be creating an object without wanting to connect to a database, so you might as well make it the __construct() function & require one less function call.
    2. Make your the function require a file system path to the config file, rather than just a (I'm guessing) a directory
  • your affected_rows(), num_rows(), and insert_id() functions can all be 1 line - no need to store what you're going to return in a variable first
  • Why haven't you abstracted mysql_fetch_row()? Look at mysql_fetch_assoc() instead.
  • Create another function called __destruct() that calls your db_disconnect() function. __destruct() is automatically called when the object is destroyed, so you won't have to call db_disconnect() (and possibly free_result()) at the end of every script.

Like my comment said - I fixed your posts.

Re: My First PHP Class

Posted: Mon Jul 21, 2008 12:58 pm
by Christopher
Typically these types of classes are named something like "Mysql_Connection". Also, you might want to think of returning an object from the query() method that can be used to fetch rows.

Re: My First PHP Class

Posted: Tue Jul 22, 2008 7:53 am
by bob_b
Thanks you for the feedback. I've made some more changes based on the comments thus far:

Code: Select all

<?php
 
/**
 * DatabaseHandler.php
 * Class for database operations. Services avialable:
 * database connection, disconnection, queries, free result, num_rows, affected_rows, last insert id.
 */
 
class DatabaseHandler {
  
  /*
   * Makes a connection to the database server and attempts to select the database.
   * Database connection parameters are located in db_config.php.
   * Triggers an error on failure. Returns the database connection handle on success.
   */  
  public function __construct() {
    
    /*
     * Include the database connection parameters.
     */
    require_once(SITE . "/" . DB_CONF_FILE);
    
    /*
     * Attempt to make a connection. Trigger an error on failure.
     * If successful attempt to select the database. Trigger an error on failure.
     * Return the connection link identifier if no errors are detected.
     */
    $this->connection = new mysqli($dbHost, $dbUser, $dbPass);
    if(mysqli_connect_errno()) {
      trigger_error("Could not connect to database server.", E_USER_ERROR);
    } else {
       $dbSelect = $this->connection->select_db($dbName);
       if(!$dbSelect) {
         trigger_error("Could not select database.", E_USER_ERROR);
       } else {
         return $this->connection;
       }
    }
  }
  
  /*
   * Issues user defined queries to the database.
   * Triggers an error on failure. Returns the result set on success.
   */ 
  public function query($sql) {
    $this->result = $this->connection->query($sql);
    if(!$this->result) {
      trigger_error("Could not issue query.", E_USER_ERROR);
    } else {
      return $this->result;
    }
  }   
  
  /*
   * Returns the number of affected rows.
   */
  public function affected_rows() {
    return $this->connection->affected_rows;
  }
  
  /*
   * Returns the number of selected rows.
   */
  public function num_rows() {
    return $this->result->num_rows;
  }
  
  /* Returns the last insert id.
  */
  public function insert_id() {
    return $this->connection->insert_id;
  }
 
  /*
   * Attempts to free result memory. Tiggers an error on failure.
   * If successful attempts to disconnect from the database server.
   * Triggers an error on failure. Returns true on success.
   */
  public function __deconstruct() {
    $free = $this->result->close();
    if(!$free) {
      trigger_error("Could not free result set.", E_USER_ERROR);
      } else {
        $disconnect = $this->connection->close();
        if(!$disconnect) {
          trigger_error("Could not close database connection.", E_USER_ERROR);
        } else {
        return $disconnect;
      }
    }
  }  
}  // end class
 
?>
I'd modify it to use the mysqli library rather than the old mysql library. It lets you (or at least it seems like it to me) do things more efficiently.
- I did this. Maybe there's a benefit but Im not feeling it ... yet.
The private variables you have for storing the database credentials are superfluous - you don't use them anywhere. That's for the best anyway as there's no need to store them beyond your initial connection function.
- Okay.
I can see why you made the db_connect() function require a $site variable, but you're kind of limiting yourself there. What if, in the future, you want to put your database credentials in a different file? I recommend 2 things:
1. rename your db_connect() function to __construct() so it will be run automatically when the object is instantiated. You'll never be creating an object without wanting to connect to a database, so you might as well make it the __construct() function & require one less function call.
2. Make your the function require a file system path to the config file, rather than just a (I'm guessing) a directory
- I looked at this and made some changes. A pretty minor issue imho because if I want to rename that file I can just change the code. It's only used once. Where else would I need to call my db connection details?
your affected_rows(), num_rows(), and insert_id() functions can all be 1 line - no need to store what you're going to return in a variable first
- Yes, that makes sense. Changed that.
Why haven't you abstracted mysql_fetch_row()? Look at mysql_fetch_assoc() instead.
- I don't really see the benefit of doing this. Please can you explain why this would be desirable?
Create another function called __destruct() that calls your db_disconnect() function. __destruct() is automatically called when the object is destroyed, so you won't have to call db_disconnect() (and possibly free_result()) at the end of every script.
- Well, I added this but as far as I can see it's never called. I can go into my class and deliberately break the deconstruction function without generating an error. This:

Code: Select all

public function __deconstruct() {
    $free = $bogus->object->close();
    if(!$free) {
      trigger_error("Could not free result set.", E_USER_ERROR);
      } else {
        $disconnect = $bogus->connection->close;
        if(!$disconnect) {
          trigger_error("Could not close database connection.", E_USER_ERROR);
        } else {
        return $disconnect;
      }
    }
  }
Produces a similar output to:

Code: Select all

public function __deconstruct() {
    $free = $this->result->close();
    if(!$free) {
      trigger_error("Could not free result set.", E_USER_ERROR);
      } else {
        $disconnect = $this->connection->close;
        if(!$disconnect) {
          trigger_error("Could not close database connection.", E_USER_ERROR);
        } else {
        return $disconnect;
      }
    }
Both do absolutely nothing where I am expecting the first one to trigger an error ... What's going on and how can I test if the deconstructor is working?
Typically these types of classes are named something like "Mysql_Connection".
that's an interesting point. I do have difficulty sometimes with managing my naming conventions. However this class does more than simply connect so to my mind 'mysql_connection' would be even more wrong than 'DatabaseHandler'.

Anyway, thanks for the input.

Re: My First PHP Class

Posted: Tue Jul 22, 2008 9:44 am
by pickle
Do you not see the benefit of abastracting mysql_fetch_rows()/mysql_fetch_assoc() or do you not see the benefit of using mysql_fetch_assoc() period?

The purpose of a database abstraction class is to prevent you from having to re-visit your code if you change your database. For example, going from mysql to mysqli should have resulted in no change in your code, except just in your database class.

As for mysql_fetch_row() vs. mysql_fetch_assoc(), the latter makes it much easier to reference elements in the resulting row. mysql_fetch_row() requires you to use an index number in the row, so if you change the order in which you retrieve columns in your query, you have to change the index number in all the code that references each row. With mysql_fetch_assoc(), you use a string key, so the order doesn't matter.

Re: My First PHP Class

Posted: Tue Jul 22, 2008 11:12 am
by bob_b
I've made some more changes and I think I have the deconstructor working now. My error was that I was expecting a return value from the $this->result->free(); call but it won't return anything doh!

I tested the new deconstructor by calling it manually:

Code: Select all

$dbConn->__deconstructor();
Here's my new class:

Code: Select all

<?php
 
/**
 * DatabaseHandler.php
 * Class for basic database operations.
 * Connection, free result, and disconnection are all performed automatically.
 * Services available: query, affected_rows, num_rows, last_insert_id.
 */
 
class DatabaseHandler {
  
  /*
   * Makes a connection to the database server and attempts to select the database.
   * Database connection parameters are located in DB_CONF_FILE as defined in constants.php.
   * Triggers an error on failure. Returns the database connection handle on success.
   */  
  public function __construct() {
    
    /*
     * Include the database connection parameters.
     */
    require_once(SITE . "/" . DB_CONF_FILE);
    
    /*
     * Attempt to make a connection. Trigger an error on failure.
     * If successful attempt to select the database. Trigger an error on failure.
     * Return the connection link identifier if no errors are detected.
     */
    $this->connection = new mysqli($dbHost, $dbUser, $dbPass);
    if(mysqli_connect_errno()) {
      trigger_error("Could not connect to database server.", E_USER_ERROR);
    } else {
       $dbSelect = $this->connection->select_db($dbName);
       if(!$dbSelect) {
         trigger_error("Could not select database.", E_USER_ERROR);
       } else {
         return $this->connection;
       }
    }
  }
  
  /*
   * Issues user defined queries to the database.
   * Triggers an error on failure. Returns the result set on success.
   */ 
  public function query($sql) {
    $this->result = $this->connection->query($sql);
    if(!$this->result) {
      trigger_error("Could not issue query.", E_USER_ERROR);
    } else {
      return $this->result;
    }
  }
  
  /*
   * Returns the number of affected rows.
   */
  public function affected_rows() {
    return $this->connection->affected_rows;
  }
  
  /*
   * Returns the number of selected rows.
   */
  public function num_rows() {
    return $this->result->num_rows;
  }
  
  /* Returns the last insert id.
  */
  public function insert_id() {
    return $this->connection->insert_id;
  }
  
  /*
   * Attempts to free result memory.
   * Attempts to disconnect from the database server.
   * Triggers an error on failure. Returns true on success.
   */
  public function __deconstruct() {
    
    $this->result->free();
    
    $disconnect = $this->connection->close();
    if(!$disconnect) {
      trigger_error("Could not close database connection.", E_USER_ERROR);
    } else {
      return $disconnect;
    }
  }
  
}  // end class
 
?>
Do you not see the benefit of abastracting mysql_fetch_rows()/mysql_fetch_assoc() or do you not see the benefit of using mysql_fetch_assoc() period?
It's the abstraction I don't get. What's wrong with simply using:

Code: Select all

$dbConn = new DatabaseHandler;
 
$sql = "SELECT test FROM test ORDER by testID";
 
$result = $dbConn->query($sql);
 
while($row = $result->fetch_assoc()) {
  echo "<p><strong>" . $row['test'] . "</strong></p>";
}
If I'm wrong and abstracting the above would be of benefit to me. What would the abstraction look like? I'm new to PHP and I'm having a lot of trouble visualising some things - particularly where I don't understand the possible advantages.

Thanks again for all the help. This learning curve will be vertical soon!

Re: My First PHP Class

Posted: Tue Jul 22, 2008 11:24 am
by pickle
Ok, say you've written a... blogging tool. Thousands of lines of code. You're using your class & $result->fetch_assoc() all over the place. What happens when you want to move to SQLite or Postgres? Or go back to using the mysql library? None of those libraries have anything like $result->fetch_assoc(). You'd have to go through all your code updating every occurrence of $result->fetch_assoc().

If it was wrapped in an abstracting function, nothing would need to be done except updating the function in your class. There are number of ways this could look in your code. Mine, while probably not the cleanest looks, something like this:

Code: Select all

$result = $DB->execute($query,'purpose of query');
while($row = $DB->getData($result))
{
    //$row is now an associated array
}
Of course there's error checking blah blah blah. I've actually used that class to talk to Postgres and MySQL using the mysqli & mysql libraries. Doing so required a bit of a rewrite of the class for each library, but my code (like that shown above) didn't need to change at all.

Re: My First PHP Class

Posted: Wed Jul 23, 2008 8:49 am
by bob_b
Ok, say you've written a... blogging tool. Thousands of lines of code. You're using your class & $result->fetch_assoc() all over the place. What happens when you want to move to SQLite or Postgres? Or go back to using the mysql library? None of those libraries have anything like $result->fetch_assoc(). You'd have to go through all your code updating every occurrence of $result->fetch_assoc().
*inserts sound of penny dropping*

I've made some further changes. I'm pretty happy with it. It handles everything I need for now. Also, I took arborint's advice and my class now returns data as objects.

Code: Select all

<?php
 
/**
 * DatabaseHandler.php
 * Class for basic database operations.
 * Connection, free result, and disconnection are all performed automatically.
 * Services available: query, affected_rows, num_rows, last_insert_id.
 * Errors are triggered at critical failures and rows of data are returned as objects.
 */
 
class DatabaseHandler {
  
  /*
   * Makes a connection to the database server and attempts to select the database.
   * Database connection parameters are located in DB_CONF_FILE as defined in constants.php.
   * Triggers an error on failure. Returns the database connection handle on success.
   */  
  public function __construct() {
    
    /*
     * Include the database connection parameters.
     */
    require_once(SITE . "/" . DB_CONF_FILE);
    
    /*
     * Attempt to make a connection. Trigger an error on failure.
     * If successful attempt to select the database. Trigger an error on failure.
     * Return the connection link identifier if no errors are detected.
     */
    $this->connection = new mysqli($dbHost, $dbUser, $dbPass);
    if(mysqli_connect_errno()) {
      trigger_error("Could not connect to database server.", E_USER_ERROR);
    } else {
       $dbSelect = $this->connection->select_db($dbName);
       if(!$dbSelect) {
         trigger_error("Could not select database.", E_USER_ERROR);
       } else {
         return $this->connection;
       }
    }
  }
  
  /*
   * Issues user defined queries to the database.
   * Triggers an error on failure. Returns the result set on success.
   */ 
  public function query($sql) {
    $this->result = $this->connection->query($sql);
    if(!$this->result) {
      trigger_error("Could not issue query.", E_USER_ERROR);
    } else {
      return $this->result;
    }
  }
  
  /*
   * Returns each row of data as an object.
   */  
  public function get_data() {
    return $this->result->fetch_object();
  }
  
  /*
   * Returns the number of affected rows.
   */
  public function affected_rows() {
    return $this->connection->affected_rows;
  }
  
  /*
   * Returns the number of selected rows.
   */
  public function num_rows() {
    return $this->result->num_rows;
  }
  
  /* Returns the last insert id.
  */
  public function insert_id() {
    return $this->connection->insert_id;
  }
  
  /*
   * Attempts to free result memory.
   * Attempts to disconnect from the database server.
   * Triggers an error on failure. Returns true on success.
   */
  public function __deconstruct() {
    
    $this->result->free();
    
    $disconnect = $this->connection->close();
    if(!$disconnect) {
      trigger_error("Could not close database connection.", E_USER_ERROR);
    } else {
      return $disconnect;
    }
  }
  
}  // end class
 
?>
Thanks for all the help and advice! :)

Re: My First PHP Class

Posted: Wed Jul 23, 2008 9:40 am
by pickle
Just to be super clean, you should probably declare an object variable $connection, as you are using it throughout the class.

Re: My First PHP Class

Posted: Wed Jul 23, 2008 9:52 am
by bob_b
Should I also do that for $result as I use that quite a few times too. I'm not really clear on declaring variables in PHP classes. All the tutorials mention things like 'var $foo' but this seems optional? I understand the visibility modifiers (I think) but variable declaration seems a bit cloudy.

Am I correct in thinking that I should be using:

Code: Select all

private $connection;
private $result;
But I don't need to declare the other variables such as dbUser and dbName because they are declared in a file that I'm including. Right?

/me get's his confused hat out in readiness ...

Re: My First PHP Class

Posted: Wed Jul 23, 2008 10:00 am
by pickle
You should declare any $this->XXX variable. Using var $blah was the old PHP4 way of doing it. You're right in using the scope declarations now. What you've got there in the post above is good.

Re: My First PHP Class

Posted: Wed Jul 23, 2008 10:17 am
by bob_b
That makes perfect sense. Thanks for all your help pickle :)

Re: My First PHP Class

Posted: Wed Jul 30, 2008 2:04 pm
by matthewcl375
Instead of trigger_error, couldn't you use something like this:

Code: Select all

 
$connection = mysql_connect('localhost', 'root', 'password') or die ('Could not connect!');
 
?