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

Where causes code to be called twice #1181

Open
Deijin27 opened this issue Nov 15, 2023 · 3 comments
Open

Where causes code to be called twice #1181

Deijin27 opened this issue Nov 15, 2023 · 3 comments

Comments

@Deijin27
Copy link

Take the code

class Foo {
  num { _num }
  construct new(num) {
    _num = num
    System.print("Creating foo")
  }
}

var bar = [1, 2].map{|x| Foo.new(x) }.where{|x| x.num > 0}.toList

If run, it prints "Creating foo" 4 times. Intuitively I expect it to call only 2 times, and I tested the same thing using C# linq and it is called 2 times.

Is this possible to fix with how WhereSequence works?
At the very least it's something to look out for in case your construction is resource intensive.

@ruby0x1
Copy link
Member

ruby0x1 commented Nov 15, 2023

Interesting! Sequences are evaluated lazily, which seems to be involved.
If you resolve the map sequence first via toList it prints twice only, since the where is only on the list

var bar = [1, 2].map{|x| Foo.new(x) }.toList.where{|x| x.num > 0}.toList

@mhermier
Copy link
Contributor

mhermier commented Nov 15, 2023 via email

@pedro-w
Copy link

pedro-w commented Nov 20, 2023

I thought this was surprising too - as mhermier says it is because of the way WhereSequence calls iteratorValue on its 'upstream' sequence (at least) once to find a value that satisfies the filter and once to actually get the value (see below). Specifically iteratorValue calls back though the chain every time, in this case calling the map function on the way to the source list [1,2]

If you have two where clauses then it prints 'Creating foo' six times

var bar = [1, 2].
 map{|x| Foo.new(x) }.
 where{|x| x.num > 0}.
 where{|x| x.num < 10}.
 toList

An option would be to have WhereSequence's iterator memoise the last result but assuming you don't want to change the code, would it be possible to make this a bit clearer in the iterator protocol docs?
Thanks!

wren/src/vm/wren_core.wren

Lines 168 to 182 in c2a75f1

class WhereSequence is Sequence {
construct new(sequence, fn) {
_sequence = sequence
_fn = fn
}
iterate(iterator) {
while (iterator = _sequence.iterate(iterator)) {
if (_fn.call(_sequence.iteratorValue(iterator))) break
}
return iterator
}
iteratorValue(iterator) { _sequence.iteratorValue(iterator) }
}

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

4 participants