CODEWITHSUNDEEP
phpexceptionerror-handling
sanders
sanders

Reputation: 10888

correct way of using a throw try catch error handling

I have come accross to this function below and I am wondering wether this is the right way of using the error handling of try/catch.

public function execute()
{
    $lbReturn = false;
    $lsQuery = $this->msLastQuery;
    try
    {
        $lrResource = mysql_query($lsQuery);

        if(!$lrResource)
        {
            throw new MysqlException("Unable to execute query: ".$lsQuery);
        }
        else
        {
            $this->mrQueryResource = $lrResource;
            $lbReturn = true;
        }

    }
    catch(MysqlException $errorMsg)
    {
        ErrorHandler::handleException($errorMsg);
    }
    return $lbReturn;
}

Upvotes: 4

Views: 1529

Answers (4)

Robin Barnes
Robin Barnes

Reputation: 13541

Why have a call to ErrorHandler::handleException here anyway?

Just throw the exception, but never catch it. Then in the global initialization code for your app have a function with the following signature:

function catchAllExceptions(Exception $e)

Then call:

set_exception_handler('catchAllExceptions');

This will cause all uncaught excpetions to be passed as an argument to catchAllExceptions(). Handling all uncaught exceptions in one place like this is good, as you reduce code replication.

Upvotes: 1

Bob Fanger
Bob Fanger

Reputation: 29897

Codewise it is correct/works, However the power of try-catch is that when an Exception is thrown from deep down in one of the functions you're calling.
Because of the "stop execution mid-function and jump all the way back to the catch block".

In this case there are no deep-down exceptions therefore I would write it like this:
(Assuming there is a function "handleErrorMessage" in the ErrorHandler.)

public function execute() {
    $lsQuery = $this->msLastQuery;
    $lrResource = mysql_query($lsQuery);

    if(!$lrResource) {
         ErrorHandler::handleErrorMessage("Unable to execute query: ".$lsQuery);
         return false;
    }
    $this->mrQueryResource = $lrResource;
    return true;
}

Which I find more readable.

Upvotes: 5

BYK
BYK

Reputation: 1374

Well it is not really a good implementation since you throw the exception and you look for that exception in catch. So the answer of Visage is true.

  1. You should use a global error handler instead of a tr-catch usage like in your code.
  2. If you are not sure of the type of the error and occurance but want to continue the execution of the code although an exception had occured, then a try-catch block will help.

Upvotes: 0

pauljwilliams
pauljwilliams

Reputation: 19225

No. Throwing an exception in this case is simply a GOTO, but with a (slightly) prettier face.

Upvotes: 5

Related Questions