Proper use of try .. catch

Question

Should I be using this method of throwing errors:

if (isset($this->dbfields[$var])) {
    return $this->dbfields[$var];
} else {
    throw new FieldNotFoundException($var);
}

or this style:

try {
    return $this->dbfields[$var];
} catch (Exception $e) {
    throw new FieldNotFoundException($var);
}

...or something else altogether?

quick explanation of the code: $this->dbfields is an array. isset() checks if a variable is set, in this case, whether the array element exists.

Answer 1

The second example is bad. You're taking a lot of overhead to catch an exception when, as you demonstrate, it's just as easy to prevent the exception in the first place. Plus you also assume you know why that exception was thrown - if there was some other exception, like say an out of memory or something, you're reporting it as a "field not found" even if it wasn't.

Answer 2

Catching "Exception" is not, most of the time, considered a good practice, out of the two you displayed, I would use option 1.

Catching all exceptions may hide a different exception and mask it as a FileNotFoundException.

Answer 3

//First let's do the checks.
if(!isset($this->dbfields[$var]))
    throw new FieldNotFoundException($var);
//Now we're in the clear!
return $this->dbfields[$var];

Answer 4

I prefer the first one, but if dbfields[$var] throws something reasonable when you access a non-existent element, then I'd prefer just returning it without checking.

I don't particularly like changing the exception type unless I have a good reason -- also if you do, make sure to try to preserve the original exception and stack trace.

Answer 5

Just re-read your explanation.

I guess your method there in #1 is going to catch any exceptions that might be thrown and simply return a bool. I definitely don't like the catching of the generic exception most of the time, so #2 wouldn't be my choice.

Answer 6

"...or something else altogether?"

Neither is very good, so something else would be appropriate.

Fix version 2 to catch the correct exception, not every possible exception. Post that as option 3. I'll upvote something that catches a specific exception instead of Exception.

Answer 7

This is far from language-agnostic.

Some languages won't throw errors for accessing non-existant fields, and the preferred pattern depends a lot on the implementations of the arrays, tables, objects, etc.