View Single Post
Old 01-21-2008, 12:28 PM   #7 (permalink)
Salathe
Moderateur
RegEx Guru PHP Guru Top Contributor Advanced Programmer 
 
Salathe's Avatar
 
Join Date: Apr 2007
Posts: 1,393
Thanks: 5
Salathe is on a distinguished road
Default

Just a couple of points on your code, xenon.

Firstly, your code snippets in the first post don't show you declaring the $instance static member for the DB factory. Failing to declare that will result in a fatal error when you try to assign it the driver instance later on. Since you stated that your implementation doesn't throw any errors then I can only assume that you deleted it from your code sample in the first post to keep things short.

A useful addition for 'extensibility' as you put it, would be to make use of that argument for the DB::getInstance method. For example:
PHP Code:
if (class_exists($type.'_driver'TRUE))
{
    
// assign instance of driver

So if the class isn't available, and cannot be autoloaded, you can throw a relevant exception or fall back on something else should you choose. Someone might decide that it's fun to call DB::getInstance('wobblybobblydb') which you don't have a driver for. At the moment it is hard-coded to use the MySQL driver which I'm sure you'll agree isn't particularly extensible.

Another point is that sketch was right about your driver instantiation producing an E_STRICT warning. The associated message is, "Only variables should be assigned by reference". The simple solution, unless you don't mind bending the strict standards from time to time, is just to not use the & and let PHP do its own job of passing around references to objects.

I think there might have been another point or two that I wanted to post about but the urge to find some lunch is taking over so I can't remember what those things were! As such, I'll leave things here.
Salathe is offline  
Reply With Quote