PHP Developers Network

A community of PHP developers offering assistance, advice, discussion, and friendship.
 
Loading
It is currently Mon Sep 28, 2020 1:30 pm

All times are UTC - 5 hours




Post new topic Reply to topic  [ 24 posts ]  Go to page 1, 2  Next
Author Message
 Post subject: Bulletproof __autoloader
PostPosted: Mon Aug 14, 2006 2:57 am 
Offline
Site Administrator
User avatar

Joined: Sun May 19, 2002 10:24 pm
Posts: 6887
In my never ending quest to perfect the code that I write, I wrote this autoloader that checks if the file exists and that it is readable. Should I check that it's executable as well? Anything else I can make better?

Syntax: [ Download ] [ Hide ]
#error logging class is already initialized at this point



# init fatal_exception

class fatal_exception extends Exception {

    public function __construct($e)

    {

        parent::__construct();

        $log = e_log::getInstance(); // just an error logger

        $log->newEntry($e);

        header("Location: " . HTTP_DOMAIN . 'unavailable.html');

        exit();

    }

}







# init autoloader

function __autoload($class) {

    $f = WEBSITE_ROOT_PATH . 'includes/classes/' . $class . '.php';

    if ((file_exists($f)) && (is_readable($f)))

    {

        require_once $f;

    } else {

        new fatal_exception("FATAL: Could not init class $class");

    }

}

_________________
Image


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 5:24 am 
Offline
DevNet Resident
User avatar

Joined: Fri Apr 07, 2006 5:13 am
Posts: 1640
Location: Israel
Didn't you mean?
Syntax: [ Download ] [ Hide ]
throw new fatal_exception("FATAL: Could not init class $class");


And this:
Syntax: [ Download ] [ Hide ]
if ((file_exists($f)) && (is_readable($f)))


Could be written like this:
Syntax: [ Download ] [ Hide ]
if (file_exists($f) && is_readable($f))

It's much easier to read it like the above :wink:


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 5:26 am 
Offline
Forum Contributor
User avatar

Joined: Sun May 07, 2006 5:19 am
Posts: 101
If the file exists and it is readable, then it should logically, get included. And the point of using 'require' instead of 'include' is that the script stops execution if it cannot be included, so I am sensing some kind of contradiction here.

For example, if the file cannot be included due to some other reason (if that is possible), then the execution will stop due to the fact that 'require' failed, and yet it will not throw an execption.

Also, you are not specifying the reason why the file could not be included (such as missing, or cannot read) therefore you can make it shorter like so:

Syntax: [ Download ] [ Hide ]
<?php

# init autoloader

function __autoload($class) {

    $f = WEBSITE_ROOT_PATH . 'includes/classes/' . $class . '.php';

    $a = @include_once $f;

    if(!$a) new fatal_exception("FATAL: Could not init class $class");

}

?>


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 5:34 am 
Offline
Site Administrator
User avatar

Joined: Sun May 19, 2002 10:24 pm
Posts: 6887

_________________
Image


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 5:39 am 
Offline
Forum Contributor
User avatar

Joined: Sun May 07, 2006 5:19 am
Posts: 101
I wasn't talking about the '_once' part, I was talking about 'require' vs. 'include'. If you use 'require_once' then on failure your script will exit execution without throwing an execption. Therefore it is more logical to use '@include_once' so that even if that fails, there will be no error message and the script will continue and you can throw an exception if it failed.


Also, in fatal_exception::__construct() you have the following line:

Syntax: [ Download ] [ Hide ]
parent::__construct();


Shouldn't it be...

Syntax: [ Download ] [ Hide ]
parent::__construct($e);


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 5:41 am 
Offline
DevNet Master
User avatar

Joined: Mon Sep 19, 2005 6:24 am
Posts: 3587
Location: London
You'll get an uncaught exception error, or possibly parse error (unexpected T_NEW on line blah) as you are not throwing or assigning the new exception. Haven't tested.

The multiple parenthesis is semantics :P PEAR coding standards use each condition within parenthesis (e.g. 'if ((condition1) && (condition2))' or atleast was the last time I read them) and is also my choice of coding standard. Personal Preference :)

Also, using the line
Syntax: [ Download ] [ Hide ]
$a = @include_once $f;
may cause problems. There is a 'feature' that if you error suppress an include, it will suppress any and all errors generated by the code within the included file.


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 5:45 am 
Offline
DevNet Evangelist
User avatar

Joined: Tue May 07, 2002 9:48 am
Posts: 8391
Location: Berlin, ger
What's the point of a new exception object if it is neither used nor thrown not even returned?


Last edited by volka on Mon Aug 14, 2006 5:49 am, edited 1 time in total.

Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 5:45 am 
Offline
Forum Contributor
User avatar

Joined: Sun May 07, 2006 5:19 am
Posts: 101


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 5:51 am 
Offline
Site Administrator
User avatar

Joined: Sun May 19, 2002 10:24 pm
Posts: 6887

_________________
Image


Last edited by Benjamin on Mon Aug 14, 2006 6:12 am, edited 3 times in total.

Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 5:59 am 
Offline
DevNet Resident
User avatar

Joined: Fri Apr 07, 2006 5:13 am
Posts: 1640
Location: Israel


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 6:08 am 
Offline
Site Administrator
User avatar

Joined: Sun May 19, 2002 10:24 pm
Posts: 6887

_________________
Image


Top
 Profile  
 
PostPosted: Mon Aug 14, 2006 6:27 am 
Offline
DevNet Evangelist
User avatar

Joined: Tue May 07, 2002 9:48 am
Posts: 8391
Location: Berlin, ger
Ah, I didn't look at the constructor code of that exception. For a simple reason: I didn't expect it to do something like logging the message somewhere. Is this really necessary for constructing the object? I doubt it. In fact it's so unessential that new fatal_exception("FATAL: Could not init class $class"); only for logging a certain message looks weird to me. If this class wasn't yours would you have expected something like this? Or let me ask it in another way: Would you have thought of or searched in this way if you had been looking for a logging mechanism?
It also generates an (in my view) unnecessary strong (but weak featured) coupling with this e_log class: specified classname, only default logger available, two functions in this class required (for only one purpose) - and again: all this still only for constructing the object.
And last but not least, not everything,anything has to be logged. It's like the personal firewall constantly crying wolf. It's annoying and distracting. Why would I want an unconditional message logging if all that happend was the construction of a simple object? It also somehow contradicts the usefullness of exceptions.
(sorry, have to go right now, only short form for now)
a()->b()->c()->d(). Now an exception occurs in d(). But b() and c() do not care, all they have to do is ...not to do anything, only a() cares about that the execution somehow was aborted. In the old days without exceptions d() hat to return an error code, c() had to check it and also to return it. Same with b ...extra code ...extra code ...extra code without real meaning. Now you throw an exception in d() and since c() and b() do nothing it (more or less) directly ends in the catch block of a(), no extra code in b() and c() for "i really do not care". That's the beauty of exceptions. And that's why I wouldn't want exception handling (like logging) in the constructor of my exceptions ;)


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 6:35 am 
Offline
Site Administrator
User avatar

Joined: Sun May 19, 2002 10:24 pm
Posts: 6887
Thanks for the insight volka. I'll read that a few times and digest it. Basically I am taking a layered approach to the Exceptions. The fatal error is the error of last resort in the application. There will be other Exception classes as well. Some recoverable, some not. The fatal_exceptions are exceptions that should never happen, which is why they are all logged.

_________________
Image


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 6:44 am 
Offline
DevNet Master
User avatar

Joined: Mon Sep 19, 2005 6:24 am
Posts: 3587
Location: London
Syntax: [ Download ] [ Hide ]
$x = new fatal_exception(/*... */);
is fine, it was because in your first post you didn't throw, nor assign the new object that it would parse error. :)


Top
 Profile  
 
 Post subject:
PostPosted: Mon Aug 14, 2006 9:21 am 
Offline
Tranquility In Moderation
User avatar

Joined: Sun Feb 06, 2005 8:18 pm
Posts: 5001
Location: Indiana

_________________
- A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 24 posts ]  Go to page 1, 2  Next

All times are UTC - 5 hours


Who is online

Users browsing this forum: No registered users and 4 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Jump to:  
Powered by phpBB® Forum Software © phpBB Group