Autoloader class for PHP

Question

I created a class that I plan on using to autoload my project classes (20-30 classes) maximum. I wonder if there is anything, anything at all, that I can improve on this class to improve it any further.

Please let me know your thoughts below.

<?php
/**
 * YodaCMS, content management system.
 * 
 * @info         This class autoloads project classes. 
 * @author       Someone Someone <[email protected]>
 * @version      0.0.1
 * @package      YodaCMS
 *
 */

class autoLoader {
    private static $loadedLibrarys;

    public function __construct() {
        $loadedLibrarys = array();
    }

    public static function loadLibrarys() {
        newLibrary("testClass", new testClass());
    }

    private function newLibrary($lName, $lValue) {
        if (!$this->tryLoadLibrary($lName, $lValue)) {
            print_r('Error loading library: ' . $lName);
        }
    }

    public function tryLoadLibrary($lName, $lValue) {
        include(ROOT . '/applocation/base/classes/'.$lName.'.class.php');
        $this->loadedLibrarys[$lName] = $lValue;
        return true;
    }
}

Answer 1

I don't think that you need newLibrary and tryLoadLibrary. The names don't really make clear what the distinction is, and I can't imagine that you'd ever want to load a library but not know if it failed or not.

Also, instead of printing, I would throw an exception. That way, the calling code can handle the case (what if it's an irrelevant library? Now the user is still annoyed with a meaningless message; but what if it's a highly relevant library? Is a small message really enough?).

I would also get rid of the distinction between lName and lValue. If you follow conventions, they should be the same anyways. If they are not, it might get confusing, especially because it's not just this call, but also how they are stored in loadedLibrarys[$lName] = $lValue;,

I would also rename tryLoadLibrary to loadAdditionalLibrary. try is not really needed (we can assume that it will only try), and additional makes it more fitting with loadLibrarys. lName and lValue are also not all that clear; if I didn't read your code, I'd have no idea what l is, or what name and what a value is. libraryClassName and libraryFileName would be clearer (you can also skip the library part, as it's clear from context.

Finally, I would always check for directory traversal when including with a variable. It shouldn't really be necessary in this situation, but it's just good practice, and doesn't cost that much.