Reputation: 13
I'm currently working in PHP. I'm working on a error system for my CMS I'm building (for the fun of it). For fatal errors in my system (not in the php compiler) I created a FatalException class that extends the built in Exception class. Since these types of errors kinda halt the system anyway, I threw in a exit into the __construct.
class FatalException extends Exception{
public function __construct($message) {
exit("<h1 style='color:red;' >FATAL ERROR: $message </h1>");
}
}
So in my code I'll check something like the connection to the database and if it can't then I'll just throw a FatalException("Can't connect to database: $database_error_message"). It won't be in a try/catch block.
When I run the code and can't connect to the database, for example, all I see on the screen is a sentence in big red letters. So it works just fine, but is this bad practice/coding?
EDIT:
Actually in truth it didn't start out this way. I was originally logging the error and then exiting in the catch area, but then I thought, if all fatal errors are going to exit anyway then just put in the in constructor. Then I noticed it wasn't actually getting to the catch area it was exiting. So putting the statement in a try/catch block was kind of a moot point. Which lead to the question.
Upvotes: 1
Views: 1025
Reputation: 4996
Even you wrap exit()
into a member function of an exception, you are not using the exception here for error handling, but just the exit()
- the exception is never thrown, PHP comes to halt before that happens.
So it works just fine, but is this bad practice/coding?
Yes, it is bad practice. It also works just fine if you would have created yourself a function:
function error_exit($message) {
exit("<h1 style='color:red;' >FATAL ERROR: $message </h1>");
}
Instead think about whether you want to use Excpetions or you want to exit()
.
Upvotes: 2
Reputation: 97783
If you're going to exit()
unconditionally in a constructor, there's not really much point making it a constructor, let alone making the class an Exception
. You could more simply (and honestly) have a static function called Fatal::Die($message)
.
The point of exceptions is that they describe what the error is (by having different classes for different exceptions) and can be caught - even if only to log them to a file and bail out of the program.
What if a particular page of your site could actually cope fine without a database connection (just missing the "latest news" or something)? Then it could catch( Database_Exception $e )
and continue, while the rest of your site just falls straight into the last-ditch "oh no something went wrong" message.
A message in big red letters is also not a great error-handling mechanism for anything that someone other than you will use - either they end up seeing technical details of the error when you're not looking, or you don't know what went wrong because you hid that error.
Upvotes: 3
Reputation: 570
This is a bad idea imho. An exception needs to be caught in order to function properly. You may however create a method like ->error( $index ); that attaches to every object you make. From there you can route the error to a specific class with a try / catch block to properly handle the errors.
class TestClass
{
public function error( $index )
{
try
{
// Convert index to exception and throw it.
}
catch ( Exception $e )
{
// Handle the error
}
}
}
$a = new TestClass();
$a->error( 1000 );
Do note that this will not work for exceptions that php throws, you'll need to catch these seperately or work with execution hubs.
Upvotes: 1