Skip to content

Commit

Permalink
[ClassLoader] replaced MapFileClassLoader by MapClassLoader
Browse files Browse the repository at this point in the history
  • Loading branch information
@fabpot
fabpot committed Jul 22, 2011
1 parent 5d9bd6d commit cc07af2
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions ...ponent/ClassLoader/MapFileClassLoader.php → .../Component/ClassLoader/MapClassLoader.php
View file
Expand Up @@ -16,18 +16,18 @@
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class MapFileClassLoader
class MapClassLoader
{
private $map = array();

/**
* Constructor.
*
* @param string $file Path to class mapping file
* @param array $map A map where keys are classes and values the absolute file path
*/
public function __construct($file)
public function __construct(array $map)
{
$this->map = require $file;
$this->map = $map;
}

/**
Expand Down

6 comments on commit cc07af2

@Koc
Copy link
Contributor

@Koc Koc commented on cc07af2 Jul 22, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about extraction interface ClassLoaderInterface with methods loadClass, findFile? And removing extending UniversalClassLoader in ApcUniversalClassLoader and DebugUniversalClassLoader classes.

@stof
Copy link
Member

@stof stof commented on cc07af2 Jul 22, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Koc why would you remove the inheritance ? It would lead to code duplication

@Koc
Copy link
Contributor

@Koc Koc commented on cc07af2 Jul 22, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What code will duplicated? I think only loadClass method, so we can create

<?php

abstract class ClassLoaderAbstract implements ClassLoaderInterface
{
    public function loadClass($class)
    {
        if ($file = $this->findFile($class)) {
            require $file;
        }
    }
}

Just I think that there is no logic in "apc loader is special case (child) of universal loader". And in theory we can wrap DebugUniversalClassLoader not only on UniversalClassLoader but for MapClassLoader too.

@stof
Copy link
Member

@stof stof commented on cc07af2 Jul 22, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Apc loader calls the logic of the parent class when the location of the file is not cached yet.

@Koc
Copy link
Contributor

@Koc Koc commented on cc07af2 Jul 22, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use wrapping

<?php

class ApcUniversalClassLoader extends ClassLoaderAbstract // abstract class from above with only one method
{
    /**
     *
     * @var ClassLoaderInterface
     */
    private $loader;
    private $prefix;

    /**
     * Constructor.
     *
     * @param ClassLoaderInterface $loader Loader instance
     * @param string $prefix A prefix to create a namespace in APC
     *
     * @api
     */
    public function __construct(ClassLoaderInterface $loader, $prefix)
    {
        $this->loader = $loader;
        $this->prefix = $prefix;
    }

    public function findFile($class)
    {
        if (false === $file = apc_fetch($this->prefix . $class)) {
            apc_store($this->prefix . $class, $file = $this->loader->findFile($class));
        }

        return $file;
    }
}

@stof
Copy link
Member

@stof stof commented on cc07af2 Jul 22, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this would mean that the end-user has to wrap define a UniversalClassLoader (without registering it) and then wrapping it in the Apc loader which would be registered. the inheritance is more user-friendly here IMO.
Thus, using the ApcUniversalClassLoader to wrap the MapClassLoader does not make any sense: it adds overhead without any value. The ApcUniversalClassLoader is meant to optimize filesystem calls in the UniversalClassLoader.

Please sign in to comment.