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

Fix for the very common infinite loop issue, when forgetting to call getIterator #11

Merged
merged 3 commits into from
Feb 27, 2024

Conversation

svermeulen
Copy link
Contributor

I find that when using this library, it is very common to accidentally create infinite loops like this:

for value in lazylualinq.range(1, 10)
   :filter("v % 2 == 0") do
   print(value)
end

Instead of properly doing this:

for value in lazylualinq.range(1, 10)
   :filter("v % 2 == 0")() do
   print(value)
end

I think this is because when the for loop is given a table that has __call metamethod, it assumes it is a normal iterator function, and repeatedly calls it, causing a new iterator function to be created each iteration of the loop. Otherwise it would just produce an error instead of an infinite loop.

For some lua environments an infinite loop is much worse than just triggering an error. We can detect this mistake though, since when lua calls the given iterator function it passes in two parameter values. So we can differentiate between when the lua for-method calls it and when the user calls it themselves.

I am using select() because the parameter values are passed as nil values, so #{...} would return 0

@Henkoglobin
Copy link
Owner

Huh, interesting find! So the intended way to iterate a sequence would be to pass it to pairs, which would then call the __pairs metamethod and do the right thing from there. I'll admit that I'm basically never using that, though :D

Would you add a unit test for this? Then I'll gladly merge your PR :)

@Henkoglobin Henkoglobin self-requested a review February 26, 2024 17:46
@Henkoglobin Henkoglobin merged commit d1b5189 into Henkoglobin:main Feb 27, 2024
5 checks passed
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

Successfully merging this pull request may close these issues.

2 participants