Collecting files from a particular directory

I'm writing an object oriented package, and one problem I am trying to solve is a class that will get all PHP files in a particular directory and instantiate the classes but not abstracts or interface types.

From an object oriented perspective, is this class SOLID?

isAbstract())
                    $return[] = new $constructedClass;
            }
        }

        return $return;
    }
}

The class has one method and that method has a couple of tasks:

• get files

• parse only PHP files

• instantiate non abstracts

It seems like this breaks the single responsibility principle, but creating another class simply to do a str_replace seems pointless. What do you think?

README:

First off, I'll just focus on making this one class a tad more generic, and expandable and maintainable and... well, generally more OO. Then, though I'm not going to go into too much detail on the matter, I'll just list a few quick tips of how you could go about separating concerns even more and create an entire File namespace.

Code-review can be harsh, but I do not intend to be hurtful or patronizing. My intentions are simply to convey, to the best of my abilities my views on what might be a preferable approach. I base this on experience, as well as personal preference and. While I try to be as objective as possible, I'm only human, so it stands to reason that the code I suggest might not be to your liking. But the code listed here is untested, written off the top of my head and only serves to illustrate my point.

This class/method definitely could do with a couple of additional methods. For example: I might want to set a path, and then -at various points in time- want to get reflection classes for a given "gateway namespace".
Your code, as it now stands would imply calling the collect method with the same $path argument, which will create an all-new RecursiveDirectoryIterator instance, only to get those files I'm after.

If, however, your class would look like this:

class Collector
{
    protected $iterator = null;
    public function __construct($path = null)
    {
        if ($path) $this->setPath($path);
    }
    public function setPath($path)
    {
        $this->iterator = new \RecursiveDirectoryIterator(
            dirname(__FILE__) . $path
        );
        return $this;
    }
    public function collect($namespace, $path = null)
    {
        $return = array();
        if ($path !== null) $this->setPath($path);
        foreach(new \RecursiveIteratorIterator($this->iterator) as $file)
        {
            $class = str_replace('.php','', basename($file));
        }
    }
}

You allow users to set the base-path when creating the instance, and then use that instance to get whatever reflection classes they will be needing at any given moment.

Moreover, you can expand on this "theme" even more, by allowing them to set a property that you can use instead of using dirname(__FILE__), which -if we're honest- isn't really improving the reusability of your current code.

I also struggle to see the point in a method that only allows reflection on abstract classes only:

if((new \ReflectionClass($constructedClass))->isAbstract())

Why not create reflection classes for all matched files, and assign them to a property, so you can then use a getter where the user can specify which classes he/she needs?

class Collector
{
    const COLLECT_ABSTRACT = 0;
    const COLLECT_REGULAR = 1;
    const COLLECT_BOTH = 2;
    //properties:
    $iterator = null;
    $collection = array();
    public function getCollection($gatewayNS, $which = self::COLLECT_ABSTRACT)
    {
        if (!isset($this->collection[$gatewayNS]))
            $this->getNS($gatewayNS);
        if (!isset($this->collection[$gatewayNS][$which]))
            throw new \InvalidArgumentException('invalid offset (0,1,2 are allowed)');
        return $this->collection[$gatewayNS][$which];
    }

    private function getNS($ns)
    {
        $this->collection[$ns] = array(
            self::COLLECT_ABSTRACT => array(),
            self::COLLECT_REGULAR  => array(),
            self::COLLECT_BOTH     => array()
        );
        foreach(new \RecursiveIteratorIterator($this->iterator) as $file)
        {
            $class = $ns.str_replace('.php','', basename($file));
            if (class_exists($class))
            {
                $class = new \ReflectionClass($class);
                $this->collections[$ns][self::COLLECT_BOTH] = $class;
                if ($class->isAbstract())
                    $this->collection[$ns][self::COLLECT_ABSTRACT] = $class;
                else
                    $this->collection[$ns][self::COLLECT_REGULAR] = $class;
            }
        }
        return $this->collections[$ns];
    }
}

That should help you on your way to turn this one-method-function-in-OOP-drag thing into a bona-fide class, that you can use in future projects, as well as your current one.

This is, of course, just a start. This class still needs some more work, and actually, could do with some helper classes, too. As I said, I'm not going to deal with this in great detail, but just as a primer/list of suggestions. The first one, though, is not a suggestion, but an appeal:

Code should document itself

Looking at your code, I'm not at all aware of all the other classes you're using, lest I browse through the actual code. Namespaced class files should start with a series of use statements, that tell me what other classes they depend upon. That way, I can get a rough Idea of what classes I can expect to see in the return values, and which objects are going to be valid as arguments, too.

That, and of cour

StackExchange Code Review Q#40721, answer score: 3