Some tips on my class?

[DigiRev]

I am writing this merely as practice for writing a PHP class. I also have a friend I program with who has a hard time with queries/databases and PHP in general, so I was hoping this might make it easier for him and also keep code neater.

I am no PHP expert yet, so I am expecting there to be quite a few things that could be improved. I haven't completed the class yet, but wanted some opinions on it before I go further...in case I am doing something very wrong.

Anyway, I'm sure there are many other much better Database classes out there, but this is the code I have, followed by an example of how it is used.

Harsh criticism is welcome.

cls_database.php:

<?php

  /* ---------------------------------------------------- /*
  - clsDatabase - cls_database.php
  - Author: Daniel Elkins
  - Purpose: First class I've ever written. For personal
  -          purposes...to make my life easier. :)
  - Copyright: (c) 2008 Daniel Elkins | All Rights Reserved
  /* ---------------------------------------------------- */
  
  class Database {
    
    var $host;
    var $username;
    var $password;
    var $db_name;
    var $dbc;
    var $error_mode = 0;
    var $table;
    var $result_set;
    var $record_count;
    var $last_insert_id;
    var $query;
    
    // Constructor function.
    function Database($error_mode = 0, $p_host = NULL, $p_user = NULL, $p_pass = NULL, $p_db_name = NULL) {
      $this->error_mode = $error_mode;
      if(isset($p_host)) { $this->host = $p_host; }
      if(isset($p_user)) { $this->username = $p_user; }
      if(isset($p_pass)) { $this->password = $p_pass; }
      if(isset($p_db_name)) { $this->db_name = $p_db_name; }
      set_error_handler(array($this, 'handle_error'));
    }
    
    // Connect to database.
    function Connect() {
      $this->dbc = mysql_connect($this->host, $this->username, $this->password);
      if(!$this->dbc) {
        trigger_error('Unable to connect to database.', E_USER_ERROR);
      }
      mysql_select_db($this->db_name) or trigger_error(mysql_error(), E_USER_ERROR);
      return TRUE;
    }
    
    // Disconnect from database.
    function Disconnect() {
      if($this->dbc) {
        mysql_close($this->dbc);
        return TRUE;
      }
    }
    
    // Perform a select query and return the records.
    function Select($p_fields = "*", $p_where = NULL, $p_order = NULL, $p_sort = 'ASC', $p_limit = NULL) {
      if($this->dbc) {
        $this->query = "SELECT $p_fields FROM " . $this->table;
        if(isset($p_where)) { $this->query .= " WHERE $p_where"; }
        if(isset($p_order)) { $this->query .= " ORDER BY $p_order $p_sort"; }
        if(isset($p_limit)) { $this->query .= " LIMIT $p_limit"; }
        $this->result_set = mysql_query($this->query) or trigger_error('Error in SELECT query: "' . $this->query . '"', E_USER_ERROR);
        if($this->result_set) {
          $this->record_count = mysql_num_rows($this->result_set);
          return mysql_fetch_array($this->result_set, MYSQL_ASSOC);
        } else {
          $this->record_count = 0;
        }
      } else {
        trigger_error('Error in SELECT procedure; not connected to database!', E_USER_ERROR);
      }
    }
    
    // Performs a raw query.
    function Query() {
      if($this->dbc) {
        $this->result_set = mysql_query($this->query) or trigger_error('Error in QUERY: "' . $this->query . '"', E_USER_ERROR);
        if($this->result_set) {
          $this->record_count = mysql_num_rows($this->result_set);
        } else {
          $this->record_count = 0;
        }
      } else {
        trigger_error('Error in SELECT procedure; not connected to database!', E_USER_ERROR);
      }
    }
    
    // Performs an insert query.
    function Insert($p_values) {
      if($this->dbc) {
        if(is_array($p_values)) {
          $this->query = "INSERT INTO " . $this->table . " (";
          
          foreach($p_values as $name=>$value) {
            $names .= "$name,";
            $values .= "'" . $this->escape_data($value, $this->dbc) . "',";
          }
          if(substr($names, -1) == ',') { $names = substr($names, 0, -1); }
          if(substr($values, -1) == ',') { $values = substr($values, 0, -1); }
          $this->query .= "$names) VALUES ($values)";
          $ret = mysql_query($this->query) or trigger_error('Error in INSERT: "' . $this->query . '"', E_USER_ERROR);
          if($ret) {
            $this->last_insert_id = mysql_insert_id($this->dbc);
            return TRUE;
          } else {
            $this->last_insert_id = 0;
          }
        }
      } else {
        trigger_error('Error in INSERT procedure; not connected to database!', E_USER_ERROR);
      }
    }
    
    // Escapes data.
    function escape_data($param1, $param2) {
      if(ini_get('magic_quotes_gpc')) {
        $data = stripslashes($param1);
      } else {
        $data = $param1;
      }
      
      return mysql_real_escape_string($data, $param2);
    }
    
    // Clears the result set.
    function ClearResults() {
      if($this->result_set) {
        @mysql_free_result($this->result_set);
      }
    }
    
    // Custom error handler.
    function handle_error($type, $string, $file, $line, $vars) {
      switch($type) {
        case E_USER_ERROR:
          switch($this->error_mode) {
            default:
            case 0:
              echo "Error: $string in $file on line $line<br />";
              print_r($vars);
              exit;
            case 1:
              echo "There has been an error, sorry for the inconvenience!";
              exit;
          }
        case E_USER_WARNING:
          echo "<pre><strong>Error</strong> [$type] $string<br /></pre>\n";
          break;
        case E_USER_NOTICE:
          echo "<pre><strong>Warning</strong> [$type] $string<br /></pre>\n";
          break;
      }
    }
  
  }
?>

An example of how it is used:

<?php
  require_once('cls_database.php');
  $db = New Database(0, 'localhost', 'username', 'password', 'DigiRev');
  $db->table = 'admin';
  
  if($db->Connect()) {
    echo "Connected to database!<br />";
  
    // Insert a new record.
    $fields = array();
    $fields['username'] = 'Danny';
    $fields['password'] = md5('12345678abcdefg');
    
    if($db->Insert($fields)) {
      echo 'Insert successful! ID of last insert is: ' . $db->last_insert_id;
    } else {
      echo 'There was an error inserting...';
    }
    
    unset($fields);
    // -----------------------------------------------
    
    
    // Select something from the database
    // Argument 1: name of fields to select, or * for all fields
    // Argument 2: stuff used in WHERE, ie: id=1
    // Argument 3: field to ORDER BY, ie: name
    // Argument 4: how to sort, ie: ASC or DESC
    // Argument 5: number of records to return (LIMIT)
    $fields = $db->Select('*', "username='DigiRev'", NULL, NULL, '1');
    
    // Check results.
    if($db->result_set) {
      echo 'Found ' . $db->record_count . ' record(s).<br />';
      echo 'ID: ' . $fields['id'] . '<br />';
      echo 'Name: ' . $fields['username'] . '<br />';
      echo 'Password: ' . $fields['password'] . '<br />';
      echo 'Last login: ' . $fields['last_login'];
      unset($fields);
    } else {
      echo 'No results found.';
    }
    
    $db->Disconnect();
  }
?>

Fire away...

[the182guy]

All your vars can be changed by $db->var = 'value'; but it does not make sense to allow vars like the db user, pass etc to be changed after the object has been constructed. I suggest making any variable private unless you need to be able to change it in that way.

Another example, your result_set var only needs to be read doesn't it? So it would make sense to make that private and provide a function to read the var.

Even for vars that need to be set, its good to have the raw var private/protected and provide functions to read/write to this var, that way you can process the input when setting the var and check for bad data or peform some logging.

You could also provide access to the basic mysql functions like mysql_affected_rows(), mysql_fetch_array(), mysql_fetch_rows(), mysql_fetch_assoc().

You could make an update function that is similar to your insert, which will take a table name and an associative array as input - key as the field, value as the data, and perform a normal SQL UPDATE statement.

One more idea, you could have a switch so that the class automatically escapes all input by default, but can be turned off like $db->escape_input(false);

Edit: another nice feature would be a logging system of all the queries. So you could use a private var to keep a record of all the queries, then you would be able to provide information such as number of queries executed to render a page (as seen on some forums). It might also help when debugging and tracing deleted records. You could do anything you want with that data then, such as provide a function that exports the data in CSV format for example, or just directly output it to a flat file or database table.

PS: You arn't doing anything very wrong!