CODEWITHSUNDEEP
phpemailerror-handling
ryryan
ryryan

Reputation: 3928

More professional error handling

I have a contact form and I handle errors by checking each field one-by-one with an "if" statement. I find this hard and I can't seem to find a better/more productive way to get them working. I would also like a heading saying "Error" if one (or more) is true. But I cant get them to work with the separate "if" statements.

Here is my code:

$name = $_POST['name']; //get data from the form
$email = $_POST['email'];//get data from the form
$message = $_POST['message'];//get data from the form
if($name == ""){
echo"<p class='error'>Please enter a name.</p>";
}
if (!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$",$email)){
echo "<p class='error'>Your email address is not valid.</p>"; 
}
if($message == ""){
echo"<p class='error'>Please enter a message.</p>";
}
else{
echo"all ok, send email code...";   
}

Edit: These errors are for the validation of the form.

Upvotes: 1

Views: 676

Answers (5)

Arc
Arc

Reputation: 11286

First, don't use ereg*() functions for regular expressions matching, these are deprecated and slow. Use the preg_*() functions instead.

Also take a look at PHP's filter extension.

It provides functions to check and validate various data which you can use directly or incorporate into your own validator functions.

EDIT: Example for checking an e-mail address (see also examples on php.net):

if ( !filter_var($email, FILTER_VALIDATE_EMAIL) ) {
    echo 'Invalid e-mail "'.$email.'"';

Speaking of validation in an object-oriented manner, you could have a generic validator class with basic validation functions (where you could also integrate filter functions for a consistent API).

Additionally, if your data logically belongs together such that you can group them into objects and manage them as such, implement a validate() method in the classes of these objects that checks the object's properties by utilizing the filter functions and/or your validator class.

class Message {
    private $name;
    private $email;
    private $text;
    ...

    public function getName() {
        return $this->name;
    }

    public function setName($name) {
        $this->name = $name;
    }

    public function getEMail() {
        return $this->email;
    }

    public function setEMail($email) {
        $this->email = $email;
    }

    ...

    public function validate() {
        $errors = array();

        $nameLength = strlen($this->name);
        if ( ($nameLength === 0) or ($nameLength > self::NAME_MAX_LENGTH) )
            $errors['name'] = 'Name must be between 1 and '.self::NAME_MAX_LENGTH.' characters.';

        if ( !Validator::isEMail($this->email) )
            $errors['email'] = 'Invalid e-mail "'.$this->email.'"';

        // Other checks
        ...

        // Return TRUE if successful, otherwise array of errors
        return ( count($errors) === 0 ? true : $errors );
    }

}

Then, you could load all your form inputs into your object like this:

$message = new Message();
$message->setName($_POST['name']);
$message->setEMail($_POST['email']);
$message->setText($_POST['text']);
...
$result = $message->validate();
if ( $result === true ) {
    // Success
} else {
    // Error
    foreach ($result as $validationError) {
        // Print error
        echo htmlentities($validationError).'<br />';
    }
}

Upvotes: 1

poke
poke

Reputation: 387557

Well, you can't check all fields together as different rules may apply to each one. So what you are doing is fine, except for a mistake you made: The else part should only be echoed, when no error occurred, but in your situation the else only applies to the last if. So even if the validation of name and email fails, as long as the message one does not, the final action is done.

You could easily add some $has_error variable that contains true as soon as an error was found. Or you could use an array that holds all error messages like this:

$errors = array();
if ( empty( $name ) )
    $errors[] = 'Please enter a name.';
if ( !eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$", $email ) )
    $errors[] ='Your email address is not valid.';
if ( empty( $message ) )
    $errors[] = 'Please enter a message.';

// individual checks done
if ( sizeof( $errors ) > 0 )
{
    echo '<p class="error"><strong>Errors found:</strong><br />';
    echo implode( '<br />', $errors );
    echo '</p>';
}
else
{
    echo 'No error, everything is fine.';
}

Upvotes: 1

Philippe Carriere
Philippe Carriere

Reputation: 3720

The most professional way would be to have each field be an object with a validation method. Then you can call each field object and ask them to validate themself.

If you would like to go any further (might be overkill though) you put your objects in a list. Each of these objects are child to an interface with the validation method. So for each object in the list, you call the validation method.

Upvotes: 1

Your Common Sense
Your Common Sense

Reputation: 157839

But I cant get them to work with the separate "if" statements.

Just store error in a variable

$error = array();

if($name == ""){
  $error[] = "Please enter a name.";
}
if (!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$",$email)){
  $error[] = "Your email address is not valid."; 
}
if($message == ""){
 $error[] = "Please enter a message.";
}

if (!$error) {
  // do not echo anything here
  // but send an email
  // and use header("Location:") to redirect a user to thanks page
} else {
  echo "Error";
  foreach ($error as $line) {
    echo "<p class='error'>$line</p>";
  }
}

Upvotes: 4

Sarfraz
Sarfraz

Reputation: 382666

You are looking for validator class. Also see:

Upvotes: 1

Related Questions