Skip to content
This repository has been archived by the owner on Oct 8, 2024. It is now read-only.

Surprising/unintuitive difference in behaviour involving multiple materialized take calls #296

Open
noinkling opened this issue Apr 29, 2024 · 2 comments

Comments

@noinkling
Copy link

noinkling commented Apr 29, 2024

const nums = [1,2,3,4,5,6,7,8,9]

const iter1 = nums.values()
iter1.take(2).toArray() // [1, 2]
iter1.take(2).toArray() // [3, 4]
iter1.take(2).toArray() // [5, 6]
iter1.next() // {value: 7, done: false}

const iter2 = nums.values().map(n => n) // pretend the mapping function is doing something useful
iter2.take(2).toArray() // [1, 2]
iter2.take(2).toArray() // []
iter2.next() // {value: undefined, done: true}

At the intuitive level, I don't expect these to behave differently, it feels like an arbitrary distinction in this case.

The alternative of repeating the map (and/or flatMap, filter) call feels clunky, even if it's most likely to be done in a loop:

iter1.map(n => n).take(2).toArray()
// or
iter1.take(2).map(n => n).toArray()

With the chunking proposal in the works maybe this particular case isn't very compelling, but I thought I'd bring it up anyway.

@bakkot
Copy link
Collaborator

bakkot commented Apr 29, 2024

The cause here is that array iterators aren't closable, unlike generators. I think that was probably a mistake; they should have been, which would have made these consistent (by having the first snippet behave like the second). Unfortunately it may be too late to change now. It was never going to be practical to have the second snippet behave like the first, because iterator helpers can wrap closable things and so need to be closable.

Either way, you should avoid re-using iterators. They're really designed to be single-use. Once you call a helper method, you should stop holding on to the original thing.

@WORMSS
Copy link

WORMSS commented Sep 11, 2024

I've came here to make the same issue..
This is highly intuitive if you have ever used iterators in any other language.

function * unlimitedGenerator() {
    let i = 0
    while (true) {
        yield (i++).toString(16);
    }
}

const instanceOfUnlimited = unlimitedGenerator();
const first5Iterator = instanceOfUnlimited.take(5)
for ( const item of first5Iterator ) {
    console.log('first 5', item);
}
const next5Iterator = instanceOfUnlimited.take(5);
for ( const item of next5Iterator  ) {
    console.log('next 5', item);
}
console.log('end');

This makes the 2nd for...of loop empty..

You have to do this monstrosity to make this 'work';

function * unlimitedGenerator() {
    let i = 0
    while (true) {
        yield (i++).toString(16);
    }
}

const instanceOfUnlimited = unlimitedGenerator();
for ( let i = 0; i < 5; i++ ) {
    const item = instanceOfUnlimited.next();
    if (item.done) break;
    console.log('first 5', item.value);
}
for ( let i = 0; i < 5; i++ ) {
    const item = instanceOfUnlimited.next();
    if (item.done) break;
    console.log('next 5', item.value);
}
console.log('end');

Which I was hoping .take() would be fixing.. but instead it actually makes iterators worse by killing the source of the data.

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

No branches or pull requests

3 participants