alekitto / class-finder

Utility classes to help discover other classes/namespaces
https://alekitto.github.io/class-finder/
MIT License
27 stars 5 forks source link

Implements `Countable` interface on each Finder #14

Open llaville opened 5 months ago

llaville commented 5 months ago

Hello @alekitto

With my first report #13, I've found that ComposerFinder (but its always true to all other) is not yet able to count values of classes found (like Symfony Finder did it).

Compares both syntaxes

This one works with current version 0.4.0

$finder = new ComposerFinder();

$count = 0;
foreach ($finder as $className => $reflector) {
    printf('- %s '. PHP_EOL, $className);
    ++$count;
}
printf('> found %d class(es)' . PHP_EOL, $count);

But not yet this one

$finder = new ComposerFinder();

foreach ($finder as $className => $reflector) {
    printf('- %s '. PHP_EOL, $className);
}
printf('> found %d class(es)' . PHP_EOL, count($finder));

I suggest to fix at least the ComposerFinder code like this

final class ComposerFinder implements FinderInterface, \Countable
{

    public function count(): int
    {
        return \iterator_count($this->getIterator());
    }

}

And think about other finders, or have a common fix !

WDYT ?

alekitto commented 5 months ago

Hi @llaville, I'm not completely against it, but I've already considered to implement Countable on the finders and chose not to do so for two reasons:

  1. I don't find it so useful, as I don't really have a use case for it
  2. Iterating such finders can potentially cause side-effects and I prefer this to be explicit rather then hide the iteration under a count call. On the contrary iterator_count signals that an iterator is involved and this can possibly have side-effects.

Anyway, if there's a particularly useful (non-trivial) use case, it's ok for me to implement it.

llaville commented 5 months ago

@alekitto could you explain "Iterating such finders can potentially cause side-effects" ? (I don't see what context will lead to a side-effect)

PS: Unless you have in mind note at https://symfony.com/doc/current/components/finder.html#transforming-results-into-arrays (about iterator_to_array) but it's not about iterator_count

alekitto commented 5 months ago

iterator_count (as well as iterator_to_array) iterates the iterator to its end. In this case all the finders (except the php documentor one) include the class files and/or trigger the php autoloading mechanism which can potentially produce side-effects or raise an error.

Just including a file (and registering its classes and functions) is a side-effect IMHO. The only way to not produce any side-effect is to use a non-including finder (phpDocumentor for example which unfortunately does not support attributes, but I'm also working on a finder based on php-parser) that can read the php files without adding them to the runtime.

llaville commented 5 months ago

It's interresting, because I have the same idea, and currently working on such solution. Find classes and also other symbols based on nikic/php-parser.