CODEWITHSUNDEEP
phpexception
Rafff
Rafff

Reputation: 1518

What should be in a try block and where to return my values

While learning PHP I've came across this dillema. For example I have this function to check if a client exists in the database.

public function is_client( int $client_id ) {
    $query = "SELECT * FROM clients WHERE id='$client_id'";
    $res = DB::run($query)->fetch();
    if(empty($res)) {
        return false;
    }
    return true;
}

I wanted to add try/catch functionality so I could catch the PDO errors, and this is how it looks after:

public function is_client( int $client_id ) {
    try {
        $query = "SELECT * FROM clients WHERE id='$client_id'";
        $res = DB::run($query)->fetch();
        if(empty($res)) {
            return false;
        }
        return true;
    }
    catch(PDOException $e) {
        return $e->getMessage();
    }
}

But I was wondering, what should I put in the 'try block'? Only the queries? Should I return my values within the 'try block' or outside?

I've tried a little bit more on that so I ended up with the following:

public function is_client( int $client_id ) {
    try {
        $query = "SELECT * FROM clients WHERE id='$client_id'";
        $res = DB::run($query)->fetch();
    }
    catch(PDOException $e) {
        return $e->getMessage();
    }
    if(empty($res)) {
        return false;
    }
    return true;
}

Can you please explain to me what are the differences between those examples and which is the correct way to do it?

UPDATE after people answers

After reading your answers I did this:

public function is_client( int $client_id ) {
    $query = "SELECT * FROM clients WHERE id='$client_id'";
    $res = DB::run($query)->fetch();
    if(empty($res)) {
        return false;
    }
    return true;
}// the function returns only boolean now


try {
    if(is_client(11) === true) {
        echo 'client';
    }
    else {
        echo 'client does not exist';
    }
}
catch(PDOException $e) {
    echo $e->getMessage();
}

Upvotes: 1

Views: 33

Answers (3)

Joshua Kleveter
Joshua Kleveter

Reputation: 1769

Try/Catch

Summary

Put the try/catch block in the code that calls the function, not the function itself.

Details

The try/catch block will watch for an exception to be thrown and, if an exception is thrown, break to the catch portion of the block. Specifically in this case that catch block is watching for an instance of PDOException.

In the case of an PDOException being thrown you'll most certainly not be getting any results back from the query. That being the case it makes more sense to return the exception from the function and skip returning the results that don't exist.

Recommended Alternative

The normal preferred method for this is to set up the function to throw the PDOException back to the code that calls the function and handle the exception in a try/catch block there. This makes sure that your function is consistently returning the same type of value.

Upvotes: 1

Kep
Kep

Reputation: 5857

In your first example:

public function is_client( int $client_id ) {
    try {
        $query = "SELECT * FROM clients WHERE id='$client_id'";
        $res = DB::run($query)->fetch();
        if(empty($res)) {
            return false;
        }
        return true;
    }
    catch(PDOException $e) {
        return $e->getMessage();
    }
}

Imagine a PDO Exception is somehow thrown during your query (this would happen in the $res = DB::run($query)->fetch(); line), this would cause execution to jump directly to the catch-Block and your is_client function (which you'd normally expect to return a boolean) suddenly returns a string.

So, if you're using your is_client function like this:

if (is_client(123))
{
    echo "We have a valid client";
} else {
    echo "Client Invalid!";
}

Would actually print the valid-statement since a non-empty string (your exception error message) evaluates to true.

The correct way would either be to not catch the exception in is_client but rather later in your program flow (eg in the block where you're using is_client) or to return a valid/expected value (or throw a new/custom exception) in your function, for example:

public function is_client( int $client_id ) {
    try {
        $query = "SELECT * FROM clients WHERE id='$client_id'";
        $res = DB::run($query)->fetch();
        if(empty($res)) {
            return false;
        }
        return true;
    }
    catch(PDOException $e) {
        // Somehow write a log file with the error message $e->getMessage()

        return false; // Return false just to be save
    }
}

Upvotes: 1

Ramy Talal
Ramy Talal

Reputation: 91

I prefer the second example. If DB::run($query)->fetch() throws an exception, if(empty($res)) { won't be called either way.

Upvotes: 0

Related Questions