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

Why an iterator instead of an array? #53

Closed
Alhadis opened this issue Nov 26, 2021 · 21 comments
Closed

Why an iterator instead of an array? #53

Alhadis opened this issue Nov 26, 2021 · 21 comments

Comments

@Alhadis
Copy link

Alhadis commented Nov 26, 2021

NB: I posted this as feedback to #42, but took @devsnek's advice to open a new issue. Original comment begins below:

What's the rationale for returning an iterator instead of a plain Array? Is it performance? Or is all this just so users can return an infinite range? Because the latter seems like an edge-case to me: I honestly can't foresee any realistic scenarios where one would need an infinite range that isn't part of a generator. Consider the example in the readme:

README.md example:Simplified equivalent:
function* even(){
	for(const i of Number.range(0, Infinity))
		if(i % 2 === 0) yield i;
}
function* even(){
	for(let i = 0; ; ++i)
		if(i % 2 === 0) yield i;
}

Conversely, returning a pre-populated list of values is pretty much what I'd expect a method named .range() to return:

const digits = Number.range(0, 9);
1 === digits[1];
9 === digits.pop();

It's also objectively more ergonomic, given the majority of users will probably need to convert the result to an array anyway:

const uppercase = Number.range(65, 90).map(n => String.fromCharCode(n));
const lowercase = Number.range(97, 122).map(n => String.fromCharCode(n));
// Versus:
const uppercase = Array.from(Number.range(65, 90)).map(n => String.fromCharCode(n));
const lowercase = Array.from(Number.range(97, 122)).map(n => String.fromCharCode(n));
@bakkot
Copy link
Contributor

bakkot commented Nov 26, 2021

Even very large but finite arrays are expensive, and wanting to perform a task a large but finite number of times is not that unusual.

I note that Python's range method previously returned a list and they regretted it so much that they changed it to be an iterable in Python 3, even though they also had an iterable-based version available in Python 2. Given that precedent, I would be very strongly opposed to returning an Array.

@Alhadis
Copy link
Author

Alhadis commented Nov 26, 2021

I don't know why we're placing so much importance on what Python does/did. Unlike generators and iterators, this is a decidedly trivial feature that exists in many scripting languages, often as a rather pedestrian addition that does what it says on the tin: return a range of values.

Even very large but finite arrays are expensive

I'd argue that's a non-issue, as sparse arrays with a predetermined length (i.e., new Array(3e7);) can mitigate a lot of performance issues. Which, I'd argue, are the concerns the language's interpreter/compiler, not that of its design.

and wanting to perform a task a large but finite number of times is not that unusual.

Yes, that's why we have for loops:

for(let i = 0; i < 3e7; ++i){
	...
}

@ljharb
Copy link
Member

ljharb commented Nov 26, 2021

It’s very easy to turn an iterator into an array - spread or Array.from. It’s mostly impossible to avoid the overhead of an array when you start with that and want an iterator.

i care about python precisely zero; i think that this is what makes the most sense for JavaScript and for idiomatic JavaScript use cases.

@Alhadis
Copy link
Author

Alhadis commented Nov 26, 2021

It’s very easy to turn an iterator into an array - spread or Array.from. It’s mostly impossible to avoid the overhead of an array when you start with that and want an iterator.

The motivation for this addition is literally "because we don't have it yet™". It's clear that it's intended to be a high-level, user-centric convenience, and not something to grease the wheels of web-apps to make them run faster…

So it stands to reason we'd want to cut out as many keystrokes as possible for developers by returning a more commonly-used and useful object (which is already an iterator, and doesn't require an extra [...range] or Array.from(…).

and for idiomatic JavaScript use cases.

Please elaborate, because I'm stretched to think of any realistic use cases where the trade-off between convenience and performance makes sense.

Seriously.

@zloirock
Copy link
Contributor

zloirock commented Nov 26, 2021

Most likely, iterators helpers will be standardized before Number.range. How do you propose to do it with array as a result?

Number.range(0, Infinity).filter(it => !(it % 17)).map(it => it ** 2).take(100).toArray()

@zloirock
Copy link
Contributor

zloirock commented Nov 26, 2021

And also,

const uppercase = Number.range(65, 91).map(n => String.fromCharCode(n)).toArray();
const lowercase = Number.range(97, 123).map(n => String.fromCharCode(n)).toArray();

@Alhadis
Copy link
Author

Alhadis commented Nov 26, 2021

Most likely, iterators helpers will be standardised before Number.range. How do you propose to do it with array as a result?

I feel we're trying to solve two fundamentally different problems here:

  1. Provide an ergonomic way to populate an array with contiguous values.
    Like String.prototype.{pad,trim}{Start,End}, listing a sequence of 𝑁 numbers starting at 𝑋 is an annoying fiddly thing to do without a dedicated method.

  2. Give iterator helpers a basis upon which to compose potentially infinite iterators.
    I get it. Composition is powerful, and a lazily-loaded list of infinite values needs to come from somewhere. But I wouldn't shoehorn this responsibility into a helper function that nominally generates a finite list of values. Instead, I'd communicate the intended use more explicitly; for example, perhaps by shamelessly ripping off Ruby:

    Number.prototype.times = function*(step = 1){
    	if(this < 0) for(let i = 0; i > +this; yield i -= step);
    	else         for(let i = 0; i < +this; yield i += step);
    };
    (1000).times().filter(x => !(x % 3));
    
    Infinity.times(); // And you can see how this goes…

    Or even just

    // Returns an iterator that always returns the given value (`true` by default)
    const devZero = Boolean.always(0);
    for(let nada of devZero){
    	// … never-ending supply of fuck all
    }

My opposition to making it an iterator drops off considerably if the iterators proposal goes through, as it'd eliminate every current gripe I have with working with iterators. I was afraid bringing that proposal up would derail this discussion at best and shift goalposts at worst, since my arguments would drift from one of developer convenience to that of semantics and how overloading a utility function is poor design.

@zloirock
Copy link
Contributor

zloirock commented Nov 26, 2021

Provide an ergonomic way to populate an array with contiguous values.

Give iterator helpers a basis upon which to compose potentially infinite iterators.

It's only two of many use cases. But I agree that the most common subsets of the second use case can be done in a better way, for example, #48 or .times().

@zloirock
Copy link
Contributor

About the array-based way - see, for example, https://es.discourse.group/t/array-create/628 or https://es.discourse.group/t/provide-an-easy-way-to-create-a-new-array-filled-via-a-mapping-function/1056 - that makes sense, but it's better to implement in another proposal.

@ljharb
Copy link
Member

ljharb commented Nov 26, 2021

I also don’t have any interest in infinite iterators; the use case here isn’t only to get an array of contiguous values, it’s to operate over a numeric range. One use case is to make an array, but many others aren’t. Iterator helpers certainly make this proposal much more valuable than it would be without them.

@Alhadis
Copy link
Author

Alhadis commented Nov 27, 2021

it’s to operate over a numeric range.

Which you can still get with Array.prototype.values() or Array.prototype.entries().

I also don’t have any interest in infinite iterators;

Since you seem to be in charge of this proposal, I guess there's really nothing else to discuss here.

@Alhadis Alhadis closed this as completed Nov 27, 2021
@ljharb
Copy link
Member

ljharb commented Nov 27, 2021

On the contrary; I'm not a champion of the proposal at all, I'm just a delegate.

I'm merely representing a point of view your comments claim doesn't exist - someone who only really cares about finite iterators, doesn't particularly care what other languages do, and (at least, with iterator helpers) thinks that it wouldn't make any sense at all for this to be an array, when it's so trivial to turn an iterator into an array.

You're totally right that you can (ab)use an array solely to operate over a range, but that doesn't mean it's the cleanest or most intuitive or most performant way to achieve that.

@hax
Copy link
Member

hax commented Jan 22, 2022

Though I believe the champion and most delegates who care about this proposal would agree it shouldn't be array, but we should also notice lodash.range actually give array, and python range also support get item via index which make it very like array.

There is dilemma, on one side, we want good performance for large range (or even infinite, though some delegates think it's not the case, we could see a very very big range is just infinite in practice 😝 ), we want transformations via iterator helpers, on the other side, there are many use cases (if u check how lodash.range and python range are using in the wild) based on array-like behavior, and I also strongly worry about the footguns of iterator one-shot behavior (which is a huge divergence from array). Such dilemma make the proposal in a stalemate. Maybe we should reconsider the whole idea thoroughly.

@ljharb
Copy link
Member

ljharb commented Jan 22, 2022

lodash.range long predates iterators in the language.

@hax
Copy link
Member

hax commented Jan 23, 2022

Yeah, but I think we still need to consider it, or we just break the use cases into two categories, and make it hard for users to adopt the feature and migrate from lodash.

@ljharb
Copy link
Member

ljharb commented Jan 23, 2022

Especially with iterator helpers, i don’t think it’s much of a concern.

@bathos
Copy link

bathos commented Jan 24, 2022

hard for users to adopt the feature and migrate from lodash

When an array is what’s needed, isn’t it just a spread? I say when cause it doesn’t seem like that’s a given. Lodash-using code could already be consuming it as an iterable, and both functions do return iterables, so wouldn’t “migrating” sometimes just be identifier swaps? (assuming the same treatment of the arguments; not sure)

for (let n of _.range(0, 1));
for (let n of Number.range(0, 1));

In any case, lodash has a good number of methods with the same names as methods found in ES, but very few (are there any?) which have same signatures or the same behaviors. Sometimes the ES method existed first, too (e.g. max), so it doesn’t seem like parity was a goal or expectation.

Console output showing different results for _.assign and Object.assign (treatment of both values and keys) and _.max and Math.max (treatment of NaN), plus an odd example of lodash saying a regular expression is a function because its @@toStringTag value is "Proxy"

@hax
Copy link
Member

hax commented Jan 24, 2022

Yeah if u always use it like for (x of lodash.range()) everything is fine, but last time I pulled some random github repos which have the dependency of lodash.range, I found some usage like {data: lodash.range(...)} and use the data in some react components, or some usage like lodash.range(start, end).map(i => (<div>{i}</div>)) and if u change it to iterator semantic range, the app will give u some unexpected result (possible random result because of react fiber/concurrent mode, though I'm not sure).

So if range proposal eventually choose iterator semantic, it means React users very likely can't use such API in common cases and just bring frustration when they try to use new JS range API and found that not work as expect.

See facebook/react#20707 for more discussion.

@hax
Copy link
Member

hax commented Jan 24, 2022

so it doesn’t seem like parity was a goal or expectation.

Yeah the official APIs could have differences, people (have to) know that. But the examples u enumerate are all edge cases. I'm not sure we can say react usage (or some similar usage rely on iterable semantics) are edge cases.

@bathos
Copy link

bathos commented Jan 24, 2022

I wouldn’t personally consider the treatment of NaN in max/max an edge case, but I did select cases that had same/similar signatures and return types deliberately to illustrate what I think is a more significant danger when folks are “migrating”: behavior differences that are only apparent for specific input values.

Lodash also has same-name functions which have very different API contracts from their ES counterparts, but because those differences would generally become apparent right away if you tried to just swap em, they seem less problematic to me. “Migrations” in the first group tend to silently generate novel bugs — the sort that make it to prod. The second usually just makes bulk find-and-replace a little more tedious.

I’d have expected array vs iterator to fall closer to the second of those loose categories, but I can see how it’s probably more in the middle in practice than I thought depending on usage patterns.

@hax
Copy link
Member

hax commented Jan 27, 2022

@bathos I agree most of your comments. Yes, from the engineering side, subtle differences are the biggest enemy . And this is actually why I worry about exposing iterator-based api too much. The differences between one-time shot iterator and reusable iterable are subtle, and in many cases, u just use something one-time, so no difference, but code will change, and may refactor, such subtle diff are hard to discover and cause bug. And because the bug is only triggered in multi run, it's possible only occur in specific time. This is a little bit like bugs involve async order, which are the nightmare of many devs. 😂

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

6 participants