is this class ok? any and all advice very welcome. Would it be better to pass the config file path array into the constructor or even through the static register methodnamespace dh_mvc2\autoloaders;use dh_mvc2\classes\Config;/**** @namespace dh_mvc2\autoloaders* @uses dh_mvc2\classes\Config in class constructor to include paths defined*       via config file*      */class BasicAutoloader {    private static $_instance;    private $pathsArray = array ();    /**     *     * @access private construct to force singleton creation     * @example Basicautoloader::register( (array) $extraPaths );     * @uses dh_mvc2\classes\Config to include paths defined via config file     * @see BasicAutoloader::register     * @see Config::get_paths     */    private function __construct() {        $this->pathsArray = array_change_key_case ( ( array ) Config::get_paths (), CASE_UPPER );        self::$_instance = $this;    }    public static function getInstance() {        if (! isset ( self::$_instance )) {              new self ();        }        return self::$_instance;    }    /**     * adds paths to autoloader directory array and defines paths to their array key     * @example      * array(example_dir => c:/www/nest/example) will      * define('EXAMPLE_DIR', 'c:/www/nest/example/') nb.! trailing / added     * @param array $paths     *          paths to add to autoloader     * @return array full list of paths included     */    public function addPaths(array $paths = NULL) {        $paths = array_change_key_case ( (array) $paths, CASE_UPPER );        $this->pathsArray = array_merge_recursive ( $this->pathsArray, $paths );        foreach ( $this->pathsArray as $key => $path ) {            $path = str_replace ( \\, /, (rtrim ( $path, '/' ) . /) );            $this->pathsArray [$key] = $path;            defined ( $key ) || define ( $key, $path );        }        return ( array ) $this->pathsArray;    }    /**     * static call to register this autoloader with extra paths as optional     * paramater.     * this method creates a singleton which reads in config paths on its     * creation     *     * @param array $pathsArray     *          to add to autoloader not defined in config file     * @return \dh_mvc2\autoloaders\BasicAutoloader     */    public static function register(array $pathsArray = NULL) {        $self = self::getInstance ();        $self->addPaths ( $pathsArray );        spl_autoload_register ( array (                $self,                'loader'         ) );        return $self;    }    /**     * spl_autoload_register's autoload function     */    private function loader($className) {        // swap \ for / for namespaces to be found on unix systems        $className = str_replace ( \\, /, $className ) . '.php';        foreach ( $this->pathsArray as $key => $path ) {            $fullFilePath = constant ( $key ) . DIRECTORY_SEPARATOR . $className;            if (file_exists ( $fullFilePath )) {                require ($fullFilePath);                return;            }        }    }}My revised class file after reading Mogria's suggestions below. Have removed static singleton and sending array of paths rather than expect the class to retrieve from config object/class as beforenamespace dh_mvc2\autoloaders;/**** @namespace dh_mvc2\autoloaders*      */class BasicAutoloader {private $_pathsArray = array ();/** * creates instance of base autoloader and registers autoload function. * @param array $paths optional paths to add to autoloaders directories array */public function __construct(array $paths = NULL) {    $this->_pathsArray = $this->change_to_unix_seperator(( array ) $paths);    spl_autoload_register ( array (            $this,            'loader'     ) );}/** * ensures paths have unix directory serperators * @param array $paths * @return array */private function change_to_unix_seperator(array $paths){    foreach ($paths as $key => $path){        $path = str_replace ( \\, /, (rtrim ( $path, '/' ) . /) );    }    return (array) $paths;}/** * adds paths to autoloader directory array *  * @param array $paths *          paths to add to autoloader * @return array full list of paths included */public function addPaths(array $paths = NULL) {    $paths=$this->change_to_unix_seperator((array)$paths);    $this->_pathsArray = array_merge_recursive ( $this->_pathsArray, $paths );    return ( array ) $this->_pathsArray;}/** * spl_autoload_register's autoload function */private function loader($className) {    // swap \ for / for namespaces to be found on unix systems    $className = str_replace ( \\, /, $className ) . '.php';    foreach ( $this->_pathsArray as $key => $path ) {        $fullFilePath = $path . '/' . $className;        if (file_exists ( $fullFilePath )) {            require ($fullFilePath);            return;        }    }}}
php autloader class reviews welcome
php;object oriented
Generally your class relies on multiple global states and is therefore tightly coupled with the environment.For instance:private function __construct() {    $this->pathsArray = array_change_key_case ( ( array ) Config::get_paths (), CASE_UPPER );    self::$_instance = $this;}Instead of supplying the configuration as an Parameter to the constructor you simply call one specific implementation of Config. What if you later have classes like FileConfig which loads the configuration from a file, or DatabaseConfig which loads the configuration from the Database or some kind, You'd need to change Config everywhere in your source code.One way to solve this isse (if you need/want to) is by using Dependency Injection:You simply define an interface for which every Configuration class needs to implement, like this:interface Config {  public function get($key); // returns a config value  // more ...};Then you simply say in your class that you need an object which implements the Config interface like this.private function __construct(Config config) {    $this->pathsArray = array_change_key_case ( ( array ) config->get('paths'), CASE_UPPER );    // code ...}An other issue which you may want to rethink is, that your Autoloader class is an Singleton. This creates the following issues:Every other class which uses the Autoloader via Autoloader::getInstance is coupled to this specific implementation.You may want to have multiple instances, if you wan't two different objects for different directories.You can't control the lifetime of the Object. Once registered every class which which is required goes through this autoloader, so you can't turn it off or on.Why do you create these constants? It's not really required by your class and it simply pollutes the global namespace. And if other classes use these then they are dependent on the fact that the method which creates these constant gets executed before the classes get used.defined ( $key ) || define ( $key, $path );An other little issue is that you're not consistent with using DIRECTORY_SEPARATOR, either use / everywhere (because they work on windows & unix) and deal with the fact that some paths on windows may look ugly because they have mixed / and \ in it. Or use DIRECTORY_SEPARATOR everywhere and type a bit more.