Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Depending on your directory structure you may have name spaced class files in different directories (that isn't really part of the structure, i.e. lib/ or core/).

Symfony ClassLoader is awesome, but I wanted something lightweight. That just solves the problem above, so inspired by other frameworks implementations I did this class.

Yes, it uses PSR-0.

I'd like some improvements on how I can get a better structure, use best practises and just make the class loader get as good as it can be.

<?php
namespace MyPro;

class Autoloader
{
    /**
     *
     * @var array 
     */
    protected $directories = array();

    public function loadClass($class)
    {
        if ($class[0] == '\\') {
            $class = substr($class, 1);
        }

        $class = str_replace(array('\\', '_'), DIRECTORY_SEPARATOR, $class). '.php';

        foreach ($this->directories as $directory)
        {
            if (file_exists($path = $directory . DIRECTORY_SEPARATOR . $class))
            {
                require_once $path;

                return true;
            }
        }
    }

    public function register()
    {
        spl_autoload_register(array($this, 'loadClass'));
    }

    public function addDirectories($directories)
    {
        $this->directories = (array) $directories;
    }

    public function getDirectories()
    {
        return $this->directories;
    }
}

Example Use:

require_once __DIR__ . '/lib/MyPro/Autoloader.php';

$autoloader = new MyPro\Autoloader();
$autoloader->addDirectories(array(
    __DIR__ . '/controllers',
    __DIR__ . '/lib',
));
$autoloader->register();

Example Controller:

<?php
namespace Controller;

class Test extends MyPro\Controller
{
    public function index()
    {
       echo 'hello!';
    }
}

--- Is this correct use of namespaces (if you thinking this as some sort of framework)

share|improve this question
    
please define "lightweight" –  Rob Apodaca Jun 16 '13 at 20:14

2 Answers 2

One minor improvement would avoid an extra string concatenation while looping over the directories.

public function loadClass($class)
{
    $class = str_replace(array('\\', '_'), DIRECTORY_SEPARATOR, $class). '.php';

    if ($class[0] != DIRECTORY_SEPARATOR) {
        $class = DIRECTORY_SEPARATOR . $class;
    }

    foreach ($this->directories as $directory)
    {
        if (file_exists($path = $directory . $class))
        ...

Note that this assumes a single-character directory separator.

My only quibble with the class, and I understand you want something lightweight, is that there's no checking of the array passed to addDirectories. This shouldn't be too much of a problem since it will fail pretty quick if the parameter isn't an array or contains non-strings, but if that comes later during the execution the error will be harder to spot. I think you'll get a simple "class not found" error.

Oh, and the method should be named setDirectories since it removes any existing directories, or it should append them to the existing array.

share|improve this answer

You have a protected member within your class - $directories - which would normally contain the directories that the autoloader looks within when resolving a namespaced class to a file. When the loadClass() method executes, it iterates through this array. This isn't a PSR-0 valid principle in my opinion, as it does not account for the current directory tree structure or anything within the include path.

When you are adding extra paths that the autoloader should do resolving with, what you're technically doing is changing the include path for the class. A more elegant way to promote this behavior is well, to actually amend the include path:

/**
 * Adds an individual directory to the include path
 * @param string $dir
 */
addDirectory($dir) {
    // check for a possible duplicate
    if (!strstr(get_include_path(), $dir)) {
        set_include_path(get_include_path() . PATH_SEPARATOR . $dir);
    }
}

/**
 * Adds multiple directories to the include path
 * @param array $dirs
 */
addDirectories(array $dirs) {
    foreach($dirs as $dir) {
        $this->addDirectory($dir);
    }
}

Now, when a directory is added, it's actually added to the actual include path for the PHP interpreter for the entire lifetime of the current script's execution. This may not be most efficient way to do it, but it is lightweight. Note that I've included two methods: One for single entires and one for multiple ones. This simplifies things, and creates a relationship within your own class. The getters can also be amended to perform similar logic, but I cannot see why one would need to have such methods.

You'll also need to modify your loadClass() method to incorporate this change:

loadClass($class) {

    $class = str_replace(array('\\', '_'), DIRECTORY_SEPARATOR, ltrim($class, '\\')). '.php';

    if (!file_exists($class)) {
        throw new RuntimeException("Unable to load $class");
    }

    require_once $class;
}

As you'll notice, I've done the following to improve your method:

  • Removed the if-statement that looks for a namespace separator at the first index of the $class variable. This is unneeded for the scope of this method and is replaced with ltrim($class, '\\').
  • Removed the foreach loop: as stated above, the include path has been modified to include the hinted directories. The autoloading procedure will now also look throughout the entire include path as intended for include.
  • Modified the if-statement that checks for file existence. Please note that this does not attempt to differentiate directories on *nix filesystems. If the file does not exist, a RuntimeException is thrown. This is a better practice than just doing nothing. The return statement has also been removed, since you'll likely never need or be able to receive it.

See:

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.