CODEWITHSUNDEEP
phpexceptiontry-catch
anlogg
anlogg

Reputation: 1090

Use try/catch as if/else?

I have a class that accepts user ID when instantiated. When that ID does not exist in the database, it will throw an exception. Here it is:

class UserModel {
    protected $properties = array();

    public function __construct($id=null) {
        $user = // Database lookup for user with that ID...
        if (!$user) {
            throw new Exception('User not found.');
        }
    }
}

My client code looks like this:

try {
   $user = new UserModel(123);
} catch (Exception $e) {
   $user = new UserModel();
   // Assign values to $user->properties and then save...
}

It simply tries to find out if the user exists, otherwise it creates a new one. It works, but I'm not sure if it's proper? If not, please provide a solution.

Upvotes: 0

Views: 897

Answers (4)

Guilherme Sehn
Guilherme Sehn

Reputation: 6787

As @VictorEloy and @AIW answered, exceptions are not recommended for flow control.

Complementing, in your case, I would probably stick with a static method for finding existing users that would return an instance of UserModel if it finds, or null in case it does not. This kind of approach is used in some ORM libraries like Active Record from Ruby on Rails and Eloquent from Laravel.

class UserModel {
    protected $properties = array();

    public static function find($id) {
        $user = // Database lookup for user with that ID...

        if ($user) {
            return new UserModel($user); // ...supposing $user is an array with its properties
        } else {
            return null;
        }
    }

    public function __construct($properties = array()) {
        $this->properties = $properties;
    }
}

$user = UserModel::find(5);

if (!$user)
    $user = new UserModel();

Upvotes: 1

Victor Ribeiro da Silva Eloy
Victor Ribeiro da Silva Eloy

Reputation: 4159

No this isn't proper, try catch blocks should be used to handle code where anomalous circunstaces could happen. Here you're just checking if the user exists or not so the best way to achieve this is with a simple if else.

from wikipedia definition of programing expception:
"Exception: an abnormal event occurring during the execution of a 
routine (that routine is the "recipient" of the exception) during its execution. 
Such an abnormal event results from the failure of an operation called by 
the routine."

Upvotes: 2

Ry-
Ry-

Reputation: 224857

That seems like a proper behaviour as long as it doesn’t throw when $id is null (that way, you can assume a new one is to be created).

In the case of the code that uses your class, if you’re going to be inserting it with that same ID later, just insert it with that ID to begin with without checking – it could be possible, though unlikely, that something happens between the check and the insertion. (MySQL has ON DUPLICATE KEY UPDATE for that.)

Upvotes: 1

Al W
Al W

Reputation: 7713

It is debatable, but I'm going to say this is NOT proper. It has been discussed before. See Is it "bad" to use try-catch for flow control in .NET?

Upvotes: 1

Related Questions