Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements Countable interface on each Finder #14

Open
llaville opened this issue Mar 21, 2024 · 4 comments
Open

Implements Countable interface on each Finder #14

llaville opened this issue Mar 21, 2024 · 4 comments

Comments

@llaville
Copy link

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
Copy link
Owner

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
Copy link
Author

llaville commented Mar 21, 2024

@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
Copy link
Owner

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants