starky
starky

Reputation: 11

Throwing Exceptions

I have some question and also would to get some feedback on the way I write my code for this function; It validates if a given email already exist in the database and returns a boolean value.

  /**
   * Validate user existence by email
   *
   * @param string $email
   *
   * @return boolean
   */    
public static function validateUserByEmail($email)
{
    if(!filter_var($email, FILTER_VALIDATE_EMAIL)){
        throw new InvalidFormatException('An invalid email has been provided!', 100);
    }

    try {
        $connection = new Database();
    } catch(DatabasePDOException $e) {
        throw $e;
    }

    if($connection){
        $sql = 'SELECT count(1) AS existence
        FROM userprofile WHERE ename = ?';  
        $params = array($email);
        $response = $connection->selectPDOStatement($sql, $params);
    }

    if(isset($response)){
        return !!$response ? !!$response[0]['existence'] : false;
    } else {
        return false;
    }
}

My concerns/questions are as follows:

1) Should I throw an exception in the event of an invalid email format,

if(!filter_var($email, FILTER_VALIDATE_EMAIL)){
    throw new InvalidFormatException('An invalid email has been provided!', 100);
}

or should I just return a value?

if(!filter_var($email, FILTER_VALIDATE_EMAIL)){
    return false;
}

2) If I am throwing an exception but I am not catching them in the parent functions, am I doing it wrongly?

public function parentFunction($string){
    if(validateUserByEmail($string)){
        // do something
    }
}

3) Or is the way the function is written completely unnecessary?

Upvotes: 0

Views: 51

Answers (1)

Sergey Chizhik
Sergey Chizhik

Reputation: 617

  1. Why not? If you need to get more details of happened instead of simple true/false - it's normal. Also you could return error reason by reference. Benefit - you don't need to try/catch block, and higher level can get details. For example.
public static function validateUserByEmail($email, &$error_reason)
{
    $result = true;

    if(!filter_var($email, FILTER_VALIDATE_EMAIL)){
        $result = false;
        $error_reason = 'InvalidEmail';
    }

    ... 

    return $result;
}
  1. If you haven't exceptions handler on higher level - it's bad. In case, if you write user-oriented app, you should to know - user don't want your exceptions. :) I can say more - they don't want to see any errors. But always you should provide them human-readable message. If you write some app for yourself - catching exceptions have lower weight. But good message, that explain what happened will be better.

Upvotes: 1

Related Questions