Throw an exception in a function or how to do desc

2019-07-28 04:12发布

I am currently using MySQLi prepared statements for my database handling. Because MySQLi prepared statements only throw an error when the connection is wrong, I am forced to check for errors myself and to throw them myself too. In PDO (which I am going to use in the future because I'm convinced now it works way better) there is a much better error handling possible because PDO does throw errors as expected which you can catch with PDOException.

But for now, I am stuck with MySQLi, so I want to know if my current code is good practice? In a lot of tutorials and books I can find a similar workflow however at some places I read "Using exceptions to notify the user that some method failed is inefficient and considered bad practice." (http://www.sitepoint.com/forums/showthread.php?872057-should-i-use-1-or-many-try-catches-in-a-page-that-uses-PDO)

try{
    $user = $dataUser->getById(1);
}catch(UserNotFoundException $unfe){
    echo 'No user found with the provided id';
}catch(Exception $exc){
    //do some logging with the real exception
    ...
    //show a readable error to the website visitor
    echo "There was an error retrieving the user from the database.";
}

In my function I use something like this:
function getById($id){
    $query = "select user_id, username,firstname from users where user_id = ?";
    if ($stmt = $this->database->getConnection()->prepare($query)) {
        $stmt->bind_param('i',$id);
        if($stmt->execute()){
                throw new exception('There was an error retrieving the user by id: ' . $stmt->error);
        }
        $stmt->bind_result($userId,$username);      
        $stmt->store_result();
        $stmt->fetch();                                             
        if(($stmt->num_rows) == 1){
            $stmt->close();         
        $user = new User($username);
        $user->userId = $userId;
        $user->username = $username;
        return $user;               
        }else{
            $stmt->close();
        throw new UserNotFoundException("No user found matching the id.");
        }
    }else{          
        throw new Exception("There was a database error retrieving the user by id: " .    $this->database->getConnection()->error);
    }
}

UPDATE 1 based on the comment of 'Your Common Sense'

mysqli_report(MYSQLI_REPORT_ALL); is not working that great..

I have done some practice but it is still not clear to me what exactly is the difference between exceptions and errors. When I use PDO, I throws exceptions when the execute fails. Mysqli doesn't. You have to check it yourself and than throw it. Let's say I use a databasewrapper and call it:

  $this->db->getOne($query,'id');

Somewhere in this method $stmt->execute appears. Should I check if it succeeded and throw an exception or should I trigger an error here?

Because you are suggesting an error_handler.

Its confusing to me because if I use pdo, I should use an exception_handler right?

UPDATE 2

  1. What about ajax? I use JSON to return results. In case of an error, should I use try catch here to return json.success true or false? Or how should I handle errors here?

  2. What if I want to show more specific information? For example: when a user performs a registration and uses a username and/or e-mail that is allready registered, an error is thrown because the unique key is violated. But I don't just want to show "hey a 500 error occured", because in this case, it is important that the registrator knows whats the problem, like "this username is taken", ...

    So is it correct that in these cases, a try catch is good practice because you want to show detailed information?

I'm just confused on when to use try catch and when to let the global error handler deal the error for me. In this topic I read this:

"You can use set_exception_handler() to deal with uncaught exceptions in your custom function. The "right" way, however, would be for you to try ... catch the exception when e.g. doing a query, and use your custom function to log it accordingly." But that is considered bad practice in this topic.

2条回答
对你真心纯属浪费
2楼-- · 2019-07-28 04:38

if my current code is good practice?

No. In many ways.

First of all, mysqli managed at last with this exception thing.

mysqli_report(MYSQLI_REPORT_ALL);

will tell mysqli throw exceptions. Yet,

Using exceptions to notify the user that some method failed is inefficient and considered bad practice

Exactly. At last someone smart enough to counter all these useless tutorials and books.

//do some logging with the real exception

for the every query in your application is quite... redundant. Don't you think? Yet,

//show a readable error to the website visitor

is not that "readable". yes, it is plain English. But what a site visitor has to do with database? What user? What was "retrieving"? All this stuff is useless and confusing. Please take look ot some professionally-built sites, at this one for example. They don't burden you with any technical details, showing a generic 500 error page only.

And it has to be done not for the every operation you do in your code, but centralized, using set_error_handler().

While exceptions have to be used only to handle the error itself.

So, your try..catch block is completely redundant. In case of error an exception have to be caught by error handler, error logged and generic 500 error page shown.

throw new exception('There was an error retrieving the user by id: ' . $stmt->error);

Don't you find this piece of code, being added to the every query execution, a quite redundant again? What you really need is a database handler class to do all the dirty job.

Look how this function may look like:

function getById($id)
{
    $query = "select username from users where user_id = ?";
    $name  = $this->db->getOne($query,$id);
    return new User($name);
}

TWO rows for all the database operations.

(however your idea of creating User class is quite strange)

查看更多
放我归山
3楼-- · 2019-07-28 04:38

I agree with Serenarules when he said : "Using exceptions to notify the user that some method failed is inefficient". It's useless : the error message will be found in the error log. What can user could do with such details ? In most case, "Error 500" is good enough.

A database connection KO is a real problem that should rise an exception. But I personnaly consider that the case "No user found matching the id." should not rise any exception. Instead, this case should simply change the behavior of the application. Unless you want to listen these events in your error logs ?

查看更多
登录 后发表回答