Update the native JavaScript For Loop

[seangates]

Requirements

• Use a variable for length in the condition evaluation
• Use a += for increment

Description of the Change

This change follows guidelines for the best performance in for loops.

Alternate Designs

N/A

Benefits

• Performance

Possible Drawbacks

• Training users

Applicable Issues

N/A

[winstliu]

This change follows guidelines for the best performance in for loops.

Thanks for the PR! Could you elaborate on some of those guidelines, as well as the performance benefits you mentioned?

[seangates]

Hey there, @50Wliu! Sorry it took a while to get back.

First, the performance gain is from caching the length of the array instead of calculating it each time. So, you cache array.length in a var (in this case len) and then you don't have to do the lookup of the array's length each time, saving precious milliseconds.

Therefore:
for (var i = 0, len = array.length; i < len; i += 1) { ... }
... is faster than ...
for (var i = 0; i < array.length; i++) { ... }

Second, the urnary operator is seen as bad (esliint guideline for "no-plusplus" rule):

Because the unary ++ and -- operators are subject to automatic semicolon insertion, differences in whitespace can change semantics of source code.

Therefore i += 1 considered safer for developers.

[thomasjo]

First, the performance gain is from caching the length of the array instead of calculating it each time. So, you cache array.length in a var (in this case len) and then you don't have to do the lookup of the array's length each time, saving precious milliseconds.

Therefore:
for (var i = 0, len = array.length; i < len; i += 1) { ... }
... is faster than ...
for (var i = 0; i < array.length; i++) { ... }

In most modern JavaScript engines this should not be a concern. I haven't got the reference on hand, but in V8 this type of lookup caching is handled automatically. If you can find some recent (less than 2 years old) references that shows that this is still a problem, we can accept this change. Optionally produce a simple benchmark example that demonstrates the performance gain.

Second, the urnary operator is seen as bad (esliint guideline for "no-plusplus" rule):

Because the unary ++ and -- operators are subject to automatic semicolon insertion, differences in whitespace can change semantics of source code.

Therefore i += 1 considered safer for developers.

Since we're dealing with the final part of a for statement specification here, it is not subject to ASI, so this unary increment operator is not a problem here. There is even a specific option in ESLint for this, what they call the "afterthought". Reference: allowForLoopAfterthoughts.

[seangates]

@thomasjo Thanks for the reply. Certainly valid points.

A couple thoughts:

In most modern JavaScript engines this should not be a concern.

There are still a bunch of people not able to write code for modern browsers. Therefore, the editor should probably help them choose the syntax that has the highest probability of working. Inversely, you shouldn't assume the developer knows whether or not the browsers they're developing for have an updated V8 engine. I would be surprised if a majority knew which browsers have the V8 improvements.

There is even a specific option in ESLint for this, what they call the "afterthought".

Yup. And it's "off" by default.

[seangates]

@thomasjo Let me know if you'd like me to add any more background here.

Thanks for your feedback, though. I'm always looking to do things the correct way.

[lee-dohm]

Thanks for the effort you've invested into this.

Because snippets are so easy to override for people, end users can provide whatever format they feel is best for them. Since others may be used to the built-in snippets, we're reticent to change them unless they are flat-out wrong. For these reasons, we're generally don't accept snippet changes and we won't be accepting this change.

Thanks for your passion and interest in Atom and your work here 🙇

[seangates]

Since others may be used to the built-in snippets, we're reticent to change them unless they are flat-out wrong.

That's a bummer. I would think you're always looking to improve what you provide to the end users since they rely on your tool for "best of breed" code completion.

Do you openly push developers to override the built-in snippets, though? I mean, I know I can create new snippets, but I was not aware there was an emphasis on overriding existing snippets. That seems like a lot of work placed on developers' shoulders to know which snippets don't meet standards and then implement the best practices for various snippets.

I guess I'll just have to 🤷‍♂️ and move on.

[lee-dohm]

Do you openly push developers to override the built-in snippets, though?

It's something we often talk about, yes. However, we don't specifically mention the possibility of overriding built-in snippets in the Flight Manual. I think that would be a good addition.

[seangates]

I can 100% get behind that.

Only because you want to let people know of that option, and if they want to make sure their IDE (and sometimes team) does the right thing, they will be shown what they can do. The risk is lowered for you folks (because you can't change the core snippets) and the developers get a gain from "doing the right thing" in their projects.

Point me to the docs, and I could even try and contribute there, if you'd like.

Thanks again for taking the time for the open debate. Appreciated!

[winstliu]

The docs are available at https://github.com/atom/flight-manual.atom.io. Seems like you'd want to edit something in https://github.com/atom/flight-manual.atom.io/tree/master/content/hacking-atom/sections.