Question

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.

Was it helpful?

Solution 2

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().

OTHER TIPS

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.

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.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top