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

Question about 11.1 "Don’t use iterators" #2018

Closed
ZYinMD opened this issue Mar 2, 2019 · 10 comments
Closed

Question about 11.1 "Don’t use iterators" #2018

ZYinMD opened this issue Mar 2, 2019 · 10 comments
Labels

Comments

@ZYinMD
Copy link

ZYinMD commented Mar 2, 2019

11.1 Don’t use iterators.

Prefer JavaScript’s higher-order functions instead of loops like for-in or for-of.

Why? This enforces our immutable rule. Dealing with pure functions that return values is easier to reason about than side effects.

const numbers = [1, 2, 3, 4, 5];

// bad
let sum = 0;
for (let num of numbers) {
  sum += num;
}
sum === 15;

// good
let sum = 0;
numbers.forEach((num) => {
  sum += num;
});
sum === 15;

My question is:

Example 2 is preferred because it can't mutate its argument, however, in example 1, similar restriction can be achieved by using const:

for (const i of iterable) {
  //...
}

for (const i in iterable) {
  //...
}

So it seems for..in and for..of are not inferior than forEach in any other way, as long as I use const. Is that so?

Of course there are times when you do want to re-assign i which is convenient in forEach, but often you don't.

@ljharb
Copy link
Collaborator

ljharb commented Mar 2, 2019

const isn’t “constant value”, it’s “constant reference”, so you can still mutate the value that way.

The issue is that loops themselves are inherently inferior ways to express the intention of your code. They’re you telling the computer (and future readers) how to iterate, instead of you telling the computer what iteration you want done. Loops should be avoided period, not just because of mutation (which is both possible and avoidable with both loops and iteration methods).

@ljharb ljharb closed this as completed Mar 2, 2019
@ljharb ljharb added the question label Mar 2, 2019
@Raiondesu
Copy link

Raiondesu commented Apr 3, 2019

@ljharb, how exactly do you mutate const key = 'key';? Because in for (const key in iterable) ... this is exactly what happens - key is being assigned a string value.

The other part of an argument is not easy to reason about. What does "They’re you telling the computer ... how to iterate" mean, exactly?

@ljharb
Copy link
Collaborator

ljharb commented Apr 3, 2019

@Raiondesu in that case, the value is a string, so it's immutable. You can mutate const key = {} though, with key.whatever = 'something'. Reassignment and mutation are different.

http://qr.ae/RNMEGA and https://gist.github.com/robotlolita/7643014 may be helpful reads for understanding why loops less clearly signal intention.

@Raiondesu
Copy link

Raiondesu commented Apr 4, 2019

@ljharb, that's exactly why @ZYinMD's argument is valid:

When you write

for (const i in iterable) {
  //...
}

"i" contains a string key, not a reference to a value.

keeping this in mind, a right way to replicate Object.values(obj).forEach(...) is

for (const i in iterable) {
  const item = iterable[i];
}

"i", in this case, is immutable. You cannot reassign it.


Thanks for the links, though.

@Raiondesu
Copy link

Raiondesu commented Apr 4, 2019

Okay, having read these 2 articles, I can say 2 different things:

  1. In a "why" section of 11.1 (and almost every "controversial" section following it), "Easier to reason about" argument is deeply flawed in a sense that it does not answer the question "why" overall. What would, in fact, answer "why" - is written in the first article:

A for or while loop is a very powerful way to iterate, which means that it's ... hard to signal intent.
With higher-order functions, you specify your intent by choosing the appropriate function...

So, in the "why" segments it really should've been references to the source material of those conclusions instead of the vague "easier to reason about". I understand, however, that when you become insancely familiar with pros and cons of each approach to write iterative code, it's easy to just say "oh, it's easier to reason about". And it sounds about right as well. Though only for you (and me now) and not for people who haven't read those 2 articles. So why not formulate the reasons to be more descriptive? If yes, then would you accept a PR that reformulates the reasoning based on a particular piece of research (with link, of course)?

  1. As great as higher-order functions are, their performance and versatility are questionable at best. Sometimes, you can't just have a higher-order function for everything. After all, if clear intent and immutability were everything we, JS programmers, cared about, we would all use monads and functors everywhere already. Promises could be monads, actually, if not for the utter ignorance of so-called "pragmatic developers" in TC39.

What I'm trying to say is... this Style Guide happens to affect JS community in a big way now, as it's being referenced almost anytime when people start discussions on how they should write their code. And you, @ljharb, as a maintainer of this Style Guide, now happen to have a good chunk of responsibility for people's code writing decisions. Because not everybody understands that this style guide is, technically, Airbnb-specific in many ways, as well as your taste-specific in some ways. Because it is presented as a "reasonable approach". Should we continue to make it more reasonable then?

@ZYinMD
Copy link
Author

ZYinMD commented Apr 6, 2019

It's been a while since I submitted this question and email notification brought me back. I actually don't mind abandoning loops, it's not a big deal, but I do have one thought:

I agree that let result = data.filter(isError).map(getErrorMessage).reduce(toLog) does look nice and declarative and easy to read, but you could've done it with a single loop, now you're looping over data 3 times - I wonder if it bothers some people, the mental issue that "I'm writing my code in a slow way", and also the real performance hit, if there's any.

As far as mutation is concerned, I have thought through all scenarios of iterating over both arrays and objects using both for in and for of vs array methods, and found no case where you can get protection by using array methods compared to for (const i), because you can always mutate the argument. Protection against mutation is never the point here.

@ljharb
Copy link
Collaborator

ljharb commented Apr 7, 2019

@ZYinMD indeed that's true, but if data is less than, say, millions of items, then O(3n) is identical to O(n). It's easy to speed up clean code, and hard to clean up fast code - wait to worry about performance until it's an actual problem :-)

@Raiondesu
Copy link

Raiondesu commented Apr 7, 2019 via email

@ljharb
Copy link
Collaborator

ljharb commented Apr 7, 2019

@Raiondesu scrolling is an entirely different kettle of fish; the only correct amount of scrolling-related javascript is zero :-)

@Raiondesu
Copy link

Raiondesu commented Apr 7, 2019 via email

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

No branches or pull requests

3 participants