views:

64

answers:

3

Hi, how is everyone?

I have a conceptual problem to do with PHP classes and error handling.. Below is a basic class demonstrating my issue.

A function to create a database record exists named "domain_create". This function calls a secondary function to ensure that the domain to be created does not already exist within the database table.

In the past I have always used true or false to reflect if the function had found a record, but that creates a flaw in my logic.. Records are inserted when the domain_lu function returns false, however should be done if an error is encountered such as the select failing? Returning false would cause the create function to believe nothing has been found and continue with the create process..

My question is how should multiple states be reflected in this scenario? Is there a "best practise" for this scenario?

<?php

require_once('auth.base.class.php');
require_once('mysql.class.php');

class auth extends base
{
   public function __construct()
   {
      parent::__construct();
   }

   /*
    * User
    */

   public function domain_create($args='')
   {
      if ( domain_lu($args['dname']) === FALSE )  
      {
         return $error['Domain already in use'];
      }
   }

   /* 
    * Domain
    */

   private function domain_lu($dname)
   {
      $sql = "SELECT name FROM domain WHERE name = '$dname'";
      $this->_mysql->SQLQuery($sql); 

      if ($this->_mysql->numRow() > 0) return true; 
      else return false;
   }
}

?>
+4  A: 

You have two choices: use exceptions or define error constants, like

class auth extends base
{
   const E_OK = 0;
   const E_NOTFOUND = 1;
   const E_FAILURE = 2;
//[snip]

   private function domain_lu($dname)
   {
      $sql = "SELECT name FROM domain WHERE name = '$dname'";
      if(!$this->_mysql->SQLQuery($sql)) return self::E_FAILURE;
      if ($this->_mysql->numRow() > 0) return self::E_OK; 
      else return self::E_NOTFOUND;
   }
}
Lekensteyn
+8  A: 

You should use Exceptions. If there's a query error throw an exception stating the reason for failure. If the domain already exists, throw an exception stating that the domain exists already. Here's a very simple example:

   public function domain_create($args='')
   {
      if (!$this->domain_lu($args['dname']))  
          throw new Exception('domain already in use');
   }

   private function domain_lu($dname)
   {
      $sql = "SELECT name FROM domain WHERE name = '$dname'";
      $this->_mysql->SQLQuery($sql); // SQLQuery should throw an exception if it fails
      return ($this->_mysql->numRow() > 0);
   }

You should really create specialized exception classes so user can catch the specific exception without relying on the exception message alone, the PHP docs have more info on that. But basically doing things this way will bail out from the function as soon as an exception is thrown, and go back as long as there's code that catches the exception. Or if it's not caught anywhere, PHP will terminate the script.

reko_t
A: 

IMO, you are tackling this at the wrong place. If you need to ensure a database column cannot contain duplicate values, you should create this column with a UNIQUE key constraint:

A UNIQUE index creates a constraint such that all values in the index must be distinct. An error occurs if you try to add a new row with a key value that matches an existing row. For all engines, a UNIQUE index permits multiple NULL values for columns that can contain NULL.

DB roundtrips are usually bottlenecks in applications, so this has the advantage that you do not need to dispatch two queries against your database (one to check, one to insert). Just run your query and see if it was successful and if not, get and return the error or throw appropriate Exceptions.

Gordon
That is actually in place already.. The question is more along the lines of concept rather than implementation. I would like to apply the concepts discussed here elsewhere..
Lee
@Lee Exceptions are definitely a good approach then. I upvoted that too
Gordon
Thanks :).. i will read into them some more..
Lee