2

I have read several threads on stackoverflow about these questions:

  1. Should I use try-catch inside or around the 'query' function?
  2. Should I use try-catch on each query execution or not?

And, to be honest, I'm not sure I understand the answers - someone says yes, someone says no. So, I want to give practical example of how I do... and if you could give me answers to these two questions.

database.php:

try {
    $connection= new PDO(DB_DNS, DB_USER, DB_PASS);
    $connection->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
} catch(Exception $e) {
    echo "Error: ".$e->getMessage();
    die();
}

user.php:

class User {
    public function pronadjiSveUsere() {
        global $connection;
        $sql= $connection->prepare("SELECT * FROM users");
        $result = $sql->execute();
        return $result ;
    }

    public function find_by_id($id=0) {
        global $connection;
        $sql= $connection->prepare("SELECT * FROM users WHERE id = :id");
        $result = $sql->execute(array($id));
        return $result->fetch(PDO::FETCH_OBJ);   
    }
}

As you can see - Class user has two methods. My questions are:

1. Should I use try-catch at all in these cases? 2. If the answer is yes - should I put within methods, for example:

    public function find_by_id($id=0) {
        try {
            global $connection;
            $sql= $connection->prepare("SELECT * FROM users WHERE id = :id");
            $result = $sql->execute(array($id));
            return $result->fetch(PDO::FETCH_OBJ);
        } catch(Exception $e) {
            echo $e->getMessage();
            die();
        }
    }

or around, whenever I'm use these methods, for example:

try {
    user->find_by_id($id);
} catch(Exception $e) {
    echo $e->getMessage();
    die();
}

Thank you in advance!

PeraMika
  • 3,539
  • 9
  • 38
  • 63
  • I would put them in the function so you don't have to write it again and again when calling the method. P.S. don't use `global` in classes we have properties for that stuff. – Daan Oct 26 '15 at 15:16
  • Just a small point: PDO generates a `PDOException` Now this will fallback to a standard Exception if not processed in a `catch (PDOException $e)` but there may be times when you want to differentiate between standard Exceptions and PDOExceptions. So it is better to catch the PDOException. This also makes your code more self documentary. – RiggsFolly Oct 26 '15 at 15:20
  • Consider throwing the exception or using `trigger_error`. http://stackoverflow.com/questions/7063053/php-error-handling-die-vs-trigger-error-vs-throw-exception – Script47 Oct 26 '15 at 15:20
  • Also using `global` in a class **totally destorys the encapsulation** and is not recommended – RiggsFolly Oct 26 '15 at 15:22
  • Possible duplicate of [PDO try-catch usage in functions](http://stackoverflow.com/questions/272203/pdo-try-catch-usage-in-functions) – KishuDroid Oct 26 '15 at 15:28

2 Answers2

2

You can write books on that topic. In fact I believe there are books about this topic. So I'll keep it very brief.
For all your code snippets I'd say no - with a possible exception for the last one.
First one: database.php should not make the decision whether to go on or not. So the catch/die block might even be considered evil. Esp. when unconditionally printing the "real" error message. Some upper layer might want to try another connection instead. Or print a fancier "sorry, something went wrong. We're trying our best to bring back yaddayadda" message. Or do what it can without the database. (Maybe that's possible. It's not if database.php lets the php instance die).
Same argument for your class user. Unless there's something (intelligent) the code within user can do about the exception it should keep away from handling it. [ Printing the error message + die is not handling the exception ].
Again, it's rather unlikely (though not impossible) that the methods themselves can reasonably mitigate the problem - so, stay away from the exception; some other, upper layer has to deal with it anyway.

So, handle the execpetion only if you can really handle it.
edit: But at times it's a good idea to "convert" the actual exception into something the upper layer(s) are interested in. Then you would have a catch block that not really handles the exception but builds a new one (if possible including the original exception) and throws it.

VolkerK
  • 95,432
  • 20
  • 163
  • 226
  • Hm, just now I am watchig some Lynda tutorial and the author uses _try-catch_ **around** `require_once('../includes/database.php');` So, in his case, his database.php is without try-catch, but whenever he requires that file - he uses try-catch... because, as he says (quote): _when using pdo to connect to a database, you should always use a try catch... And, this is particularly important with databases that are password protected because an uncaught PDO exception can expose your username and password._ – PeraMika Oct 26 '15 at 15:54
  • 1
    If there's no other top-level catch block (i.e. the pdo exception would run into the default unhandled exception handler), yes. But the question would be: why is there no other exception handler? The database connection in one way or another is part of the application configuration layer and this layer should somehow handle (or delegate) the "resource not available" exception. – VolkerK Oct 26 '15 at 17:19
  • Thank you Volker, I must admit that I'm still confused about this, so I created a new thread on codereview here: [link](http://codereview.stackexchange.com/questions/108807/pdo-configuration-database-connection-and-using) - if you have time to correct that code or tell me how would you do. – PeraMika Oct 26 '15 at 17:36
1

If you need to stop the application every time your query fails and throws Exception - try to use php function set_exception_handler and remove all of your try...catch blocks for PDO in code. This function register global handler for all uncaught exception. But For example

<?php
    set_exception_handler(function ($e) {
        // Processing only PDOExceptions
        if ($e instanceof PDOException) {
            echo $e->getMessage();
            die();
        } else {
            throw $e;//do not process other exceptions
        }
    });
    ?>

This function you need to call before any of Sql queries.