CODEWITHSUNDEEP
phpmysqlfeedback
Steven
Steven

Reputation: 19425

Need some advice on error handling in PHP / mysql

I'm not sure which method is best to use when giving READABLE feedback to end user. I've read some forums, but not really gotten any wiser (or I have not understood it)

I would like to give feedback when insertion / update fails, when it's a success and when giving custom feedback (like checking if an item alread exists).

On INSERT, UPDATE, DELETE, DROP etc., the query returns either TRUE or FALSE. Therefor my result property $this->query_result should always be either true or false.

My issues:

I have added code to see what I'm doing (doing wrong)

These are the functions I use for connecting / querying the DB:

  public function connect() 
  { 

      if (!($this->conn = mysql_connect($this->host, $this->username, $this->pwd)))  {
         die("Error connecting to DB by user = " . $this->username); 
      } 

      $this->db = mysql_select_db($this->dbname,$this->conn) 
        or die("Unable to connect to database " . $this->dbname); 
  }  

  private function query($sql) 
  {
      $this->query_result = mysql_query($sql, $this->conn)or die("Unable to query local database <b>". mysql_error()."</b><br>$sql"); 

      if (!$this->query_result){ 
          die("database query failed."); 
      } else { 
          return $this->query_result; 
      }
  }

Here is my problem: I'm giving feedback on the Data Access Layer (DAL), see e.g. this:

  public function addNewPerson($formData)
  {
    $sql = "INSERT INTO my_table(`name`, `email`, `www`)";

    $sql .= " VALUES('".
      $formData['name']."','".
      $formData['email']."','".
      $formData['www']."','");

   $this->query($sql);
   return $this->query_result;
  }

By returning a text string, the return result will always be true. From what I read, I should probably have a function which handles errors / feedback.

This is what I'm currently doing with feedback in my template:

  if (isset($_POST['form_submit']))
  {

    if (isset($_POST['person_submit'])) {
      $formData = $sl->getFormData();
      $result = $myDB->addNewPerson($formData);

      if ($result == true)
      {
        echo '<script type="text/javascript" language="JavaScript">
              jQuery("#contentArea .messageWindow1").show(500);
              jQuery("#contentArea :input").click(function(){ jQuery("#contentArea .messageWindow1").hide(500); });
        </script>';
      } else {
        echo '<script type="text/javascript" language="JavaScript">
              jQuery("#contentArea .messageWindow2").show(500);
              jQuery("#contentArea :input").click(function(){ jQuery("#contentArea .messageWindow2").hide(500); });
        </script>';
      }
    }
  } 

<div id="contentArea">   
  <div class="messageWindow1"> <span class="msg"><?php echo $labelResult ?></span></div>
  <div class="messageWindow2"> <span class="msg"><?php echo $labelResult ?></span></div>
</div>

Upvotes: 0

Views: 1853

Answers (4)

Steven
Steven

Reputation: 19425

Have to answer my own example to show code snippet. Taking up on Shoan suggestion, I'm trying to extend DAL to use PDO (class DAL extends PDO). But that gives me blank screen. This is my DAL class.

class DAL { 
  protected $username; 
  protected $pwd; 
  protected $host;
  protected $dbname;
  private $conn; 
  private $db; 
  private $query_result; 

  public function __construct($cfg_file = 'nf.config') 
  { 
    $config = parse_ini_file($cfg_file);

    $this->username     = $config['db_user']; 
    $this->pwd          = $config['db_password'];
    $this->host         = $config['db_host']; 
    $this->dbname       = $config['db_name'];
  } 

  public function connect() 
  { 
      ($this->conn = mysql_connect($this->host, $this->username, $this->pwd))  
        or die("Error connecting to DB by user = " . $this->username); 

      $this->db = mysql_select_db($this->dbname,$this->conn) 
        or die("Unable to connect to database " . $this->dbname); 
  }  


  private function query($sql) 
  {
      $this->query_result = mysql_query($sql, $this->conn)
        or die("Unable to query local database <b>". mysql_error()."</b><br>$sql"); 

      if ($this->query_result){ 
          return $this->query_result; 
      }
  } 

  public function getSomeData()
  {
    $sql ="SELECT * FROM myTable";
    //Process query
    $this->query($sql);
      return $this->query_result; 
  }
}

So in my code, I just do this:
$myDB = new DAL();
$myDB->connect();
$result = $myDB->getSomeData();

But once I add 'extends PDO', my page goes blank. I'm also not able to use any try / catch / throw - it all gives me error messages.

Upvotes: 0

Shoan
Shoan

Reputation: 4078

I would use PHP5's builtin exception handling to trap errors and possibly validation errors too. For ex:

    class DatabaseException extends Exception {}
    class ValidatorException extends Exception {}

         public function connect() 
          { 

              if (!($this->conn = mysql_connect($this->host, $this->username, $this->pwd)))  {
                 throw new DatabaseException("Error connecting to DB by user = " . $this->username); 
              } 

              if(!($this->db = mysql_select_db($this->dbname,$this->conn))) { 
                throw new DatabaseException("Unable to connect to database " . $this->dbname);
 }
          }  

    //....


    public function addNewPerson($formData)
      {
        $sql = "INSERT INTO my_table(`name`, `email`, `www`)";

        $sql .= " VALUES('".
          $formData['name']."','".
          $formData['email']."','".
          $formData['www']."','");

       //If less than 2 characters, do not insert data.
       if (strlen($formData['name']) < 2)
        throw new ValidatorException( "Person not saved. Name field was to short or empty.");

       //If person already exists
       if($this->isPersonInList($formData['name']))
        throw new ValidatorException( "Person already exists!");

       //Process query
       $this->query($sql);
       return $this->query_result;
      }

In the invocation script

try {
$formData = $sl->getFormData();
$result = $myDB->addNewPerson($formData);
} catch (DatabaseException $e) {
// display $e->getMessage()
} catch (ValidatorException $e) {
//display $e->getMessage()
}

Couple of other things to point out with your script.

  1. It's better to use PDO and prepared statements.
  2. You can also use the following to determine if the string length is met.
$arr = 'Shoan';
var_dump(isset($arr[10])); //false
var_dump(isset($arr[2])); //true
  1. Filter the input for sql injection/XSS exploits before pushing it into your database or using it in your application.

Upvotes: 4

Philippe Gerber
Philippe Gerber

Reputation: 17836

Just one hint: You should use exceptions when programming OO. For example you could introduce different exceptions for your different error feedback.

class ValidationException extends Exception
{}

class DatabaseExceptionextends Exception
{}

throw ValidationException("Person not saved. Name field was to short or empty.");
throw DatabaseException("database query failed.");

Then you catch all these exception and react differently depending on the exception's type.

try {
    // ...
}
catch (ValidationException $e) {
    // ...
}
catch (DatabaseExceptionextends $e) {
    // ...
}

Upvotes: 2

DLH
DLH

Reputation: 2821

You could add a special character to the beginning of the error messages in addNewPerson(). The calling script would use string functions to detect the special character (so that it knows there's an error) and remove that character so that the message would display without it. Do you think this would work for what you're wanting?

Upvotes: 0

Related Questions