3
\$\begingroup\$

Please review my Autoloader, which is the first file included in index.php

Note that there is no resource with the same name for trait, interface and class. Each resource has it's own name. And even if some class is named "Ha" and some trait is named "Ha", one of them has specific namespace for this.

<?php
### Autoload class ###
class Autoload
{
    # The path for classes, traits or interfaces #
    private static $path  = 'private/Model/';

    # The resource files by default will be NULL #
    private static $resource_files = null;

    # Search for resource_files in recursive mode #
    private static function recursive_glob($pattern)
    {
        $resource_files = glob($pattern);
        foreach(glob(dirname($pattern).'/*', GLOB_ONLYDIR | GLOB_NOSORT) as $directory)
        {
            $resource_files = array_merge(
                $resource_files,
                self::recursive_glob($directory.'/'.basename($pattern))
            );
        }
        return $resource_files;
    }

    # Order array to start with files that has the name like the $resource or closely... #
    private static function order_resource_files($resource)
    {
        $resource_name = explode('\\', $resource);
        $resource_name = end($resource_name);

        foreach(self::$resource_files as $file_key => $file)
        {
            if(stripos($file, $resource_name) !== false)
            {
                self::$resource_files[-1] = $file;
                unset(self::$resource_files[$file_key]);

                ksort(self::$resource_files);
                self::$resource_files = array_values(self::$resource_files);
            }
        }
    }

    # Find resource #
    public static function find($resource)
    {
        # Set $resource_files if is NULL #
        if(!self::$resource_files)
        {
            self::$resource_files = self::recursive_glob(self::$path.'*.php');
        }

        # Order $resource_files by given resource #
        self::order_resource_files($resource);

        # Now that we have the $resource_files array, we can start searching #
        foreach(self::$resource_files as $file_key => $file)
        {
            # First, remove the file from array #
            unset(self::$resource_files[$file_key]);

            # Include file #
            require_once $file;

            # If resource exists now, then we can stop and search for other resources later #
            if(class_exists($resource) || trait_exists($resource) || interface_exists($resource))
            {
                break;
            }
        }
    }
}

### Check PHP version and call Autoload::find ###
if(version_compare(PHP_VERSION, '5.1.2', '>='))
{
    if(version_compare(PHP_VERSION, '5.3.0', '>='))
    {
        spl_autoload_register('Autoload::find', true, true);
    }
    else
    {
        spl_autoload_register('Autoload::find');
    }
}
else
{
    function __autoload($resource)
    {
        Autoload::find($resource);
    }
}
\$\endgroup\$
1
\$\begingroup\$

First of all you should use static methods and properties only when there is good reason to do so. Generally you should default to non-static.

  1. $path property can be const because private static doesn't say whether it is immutable or not.
  2. I think you shouldn't order resource files at all. You already have resource name. All you should do is match something like $resource.'.' and include file if such exists. Also if you did require_once i don't see the need to check for class_exists() etc.

  3. I have never written autoloader but my instinct tells me that if you failed to find resource file then you should throw exception.

  4. Altering array inside foreach is murky and its good to avoid such practices.

PS: Personally I haven't seen any php project/library utilizing hashtag comments. It might be my OCD but #...# draws all my focus so my suggestion is to use either: // for line comments and /*...*/ for block comments.

Update: What I had in mind when writing point 2 is something like this:

    foreach(self::$resource_files as $file_key => $file)
    {
        // Check if resource matches file name
        if (strpos($file, $resource.'.') !== false ) {
            unset(self::$resource_files[$file_key]);

            require_once $file;
        }
    }
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ 1. Yes. It can be const and I would do that ! 2. If you test the Autoloader you will see that there is a reason why I use class_exists etc. If I don't do that, the foreach will include all classes without any sense. This is also the reason why I reorder the array. 3. Yes, you are right ! 4. Yes, I know that altering array inside foreach sounds bad, but in my case it has a good point. If I don't delete array elements, then I will end including all files over and over. Thank you very much for your answer! \$\endgroup\$ – Andrei Feb 18 '18 at 18:21
1
\$\begingroup\$

First, I disagree with the comment of xReprisal about static. A Static method is a useful tool and spoiling it is a waste. However, you are also using a static field and it is not recommended (because it's not safe, at least on Java). I avoid (my example, see below) to use static by creating a new instance once.

For this task, I create a project (free, MIT license) that it does the auto mapper

https://github.com/EFTEC/AutoLoadOne

It generates an automap from all the classes, interfaces and traits and it produces an autoload.php. It does the autoload the most efficiently possible (I hope). So it's a two stage program.

About your code:

a) PHP 5.6 will be close to being obsolete. So, I don't think it's worth to support an old version of PHP.

b) You are scanning an entire folder (and their subfolders) each time you call a new class/interface/trait ($resource_files = glob($pattern); instead of self::resource_files i.e the recommendation of xReprisal). Even if you are doing the scan of the entire folder once, you are doing it once per customer per call. So it impacts the performance. If you can, you could cache the $self:: resource data on APCU, REDIS or even in a text field). What I did to solve it? pre-scan the folder once and for all by generating the class:

The class has two arrays, one if for map classes that are specials such as classes in different folders or classes in the same .php file. And the second array has a map of namespaces and folders.

It is how a generated autoload.php looks:

<?php
/**
 * This class is used for autocomplete.
 * Class _AutoLoad
 * @noautoload it avoids to index this class
 * @generated by AutoLoadOne 1.3 generated 2018/07/06 05:39:35
 * @copyright Copyright Jorge Castro C - MIT License. https://github.com/EFTEC/AutoLoadOne
 */
class _AutoLoad
{
    var $debug=false;
    private $_arrautoloadCustom = array(
        'MyProject\Connection' => '/../test/folder/multiplenamespace.php',
        'AnotherProject\Connection' => '/../test/folder/multiplenamespace.php',
        'MyProject\Connection2' => '/../test/folder/multiplenamespace2.php',
        'AnotherProject\Connection2' => '/../test/folder/multiplenamespace2.php',
        'ClassWithoutNameSpace' => '/../test/folder/subfolderalt/ClassWithoutNameSpace.php',
        'folder\subfolder\CustomClass' => '/../test/folder/subfolderalt/CustomClass.php'
    );
    private $_arrautoload = array(
        'folder' => '/../test/folder',
        'folder\subfolder' => '/../test/folder/subfolder',
        'sub\sub\sub' => '/../test/folder/subfolder/subsubfolder'
    );
    /**
     * _AutoLoad constructor.
     * @param bool $debug
     */
    public function __construct($debug=false)
    {
        $this->debug = $debug;
    }
    /**
     * @param $class_name
     * @throws Exception
     */
    public function auto($class_name) {
        // its called only if the class is not loaded.
        $ns = dirname($class_name); // without trailing
        $ns=($ns==".")?"":$ns;        
        $cls = basename($class_name);
        // special cases
        if (isset($this->_arrautoloadCustom[$class_name])) {
            $this->loadIfExists($this->_arrautoloadCustom[$class_name] );
            return;
        }
        // normal (folder) cases
        if (isset($this->_arrautoload[$ns])) {
            $this->loadIfExists($this->_arrautoload[$ns] . "/" . $cls . ".php");
            return;
        }
    }

    /**
     * @param $filename
     * @throws Exception
     */
    public function loadIfExists($filename)
    {
        if((@include __DIR__."/".$filename) === false) {
            if ($this->debug) {
                throw  new Exception("AutoLoadOne Error: Loading file [".__DIR__."/".$filename."] for class [".basename($filename)."]");
            } else {
                throw  new Exception("AutoLoadOne Error: No file found.");
            }
        }
    }
} // end of the class _AutoLoad
if (defined('_AUTOLOADONEDEBUG')) {
    $_autoLoad=new _AutoLoad(_AUTOLOADONEDEBUG);
} else {
    $_autoLoad=new _AutoLoad(false);
}
spl_autoload_register(function ($class_name)
{
    global $_autoLoad;
    $_autoLoad->auto($class_name);
});
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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