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

Wrong return type iterable of UrlProviderInterface::generate #258

Open
mbenoukaiss opened this issue Oct 14, 2024 · 0 comments
Open

Wrong return type iterable of UrlProviderInterface::generate #258

mbenoukaiss opened this issue Oct 14, 2024 · 0 comments

Comments

@mbenoukaiss
Copy link

mbenoukaiss commented Oct 14, 2024

UrlProviderInterface::generate is typed as returning an iterable which I believe is wrong, an array is iterable, but a Generator is also an iterable. However the value returned by generate is sent to array_merge which only accepts arrays.

Seeing the return type of UrlProviderInterface::generate made me believe I could use yield and return a generator in my implementation but it is not the case and will give the error array_merge(): Argument #1 must be of type array, Generator given.

I see three possible solutions :

  • Change UrlProviderInterface::generate signature to return array
  • Call iterator_to_array in SitemapBuilder::generate if the value is not an array
  • Use AppendIterator instead of merging arrays to make use of the iterators returned, which if used properly could lead to less memory usage since we don't have to load the whole URLs array in memory (however I don't know about CPU performance). But is it worth for a command ? I don't know much about the codebase but it could make it more complex for negligible gains.

Whatever your decision is I can gladly make a PR

@mbenoukaiss mbenoukaiss changed the title Return type iterable of UrlProviderInterface::generate not precise enough Wrong return type iterable of UrlProviderInterface::generate Oct 14, 2024
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

1 participant