Skip to content

Comments

Avoid classical for if for...of when looping all object of an Array-like object (part #4)#19544

Merged
Josh-Cena merged 24 commits intomdn:mainfrom
teoli2003:fix-classical-for-4
Sep 2, 2022
Merged

Avoid classical for if for...of when looping all object of an Array-like object (part #4)#19544
Josh-Cena merged 24 commits intomdn:mainfrom
teoli2003:fix-classical-for-4

Conversation

@teoli2003
Copy link
Contributor

Classical for loops are error-prone (setting the right condition, not mixing indices…) and difficult to read when used to loop on all elements of an Array-like structure.

This PR changes some occurrences to use the modern for…of, or .forEach() in some occurrences.

Note that:

  • I use for…of if I don't need the index
  • I use .forEach() when I do, or on an object where for…of may not work.
  • There were a couple of cases where the loop copied a _typed array into a new one; I used UintXYArray.from() instead.

@teoli2003 teoli2003 requested a review from a team as a code owner August 14, 2022 10:59
@teoli2003 teoli2003 requested review from sideshowbarker and removed request for a team August 14, 2022 10:59
@github-actions github-actions bot added the Content:WebAPI Web API docs label Aug 14, 2022
@github-actions

This comment was marked as resolved.

@sideshowbarker sideshowbarker removed their request for review August 14, 2022 12:19
@rubiesonthesky
Copy link
Contributor

You probably know this , but using eslint-plugin-unicorn's rule no-for-loop and --fix will correct these easily for you and fix the variable names. Of course you need then to review them one by one, if they are correct or not. It also converts to for...of using .entries() so you don't need to even do that manually. (And then you can change it to forEach if you want). You probably needs still find the existing ones with regex.

@teoli2003 teoli2003 requested review from Elchi3, Josh-Cena and rubiesonthesky and removed request for rubiesonthesky August 30, 2022 12:31
Copy link
Contributor

@rubiesonthesky rubiesonthesky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look good to me.

Copy link
Member

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this LGTM

@Josh-Cena Josh-Cena merged commit 94dfec4 into mdn:main Sep 2, 2022
goshdarnheck pushed a commit to goshdarnheck/content that referenced this pull request Sep 7, 2022
…ike object (part mdn#4) (mdn#19544)

* Avoid classical for if for...of when looping all object of an Array-like object

* Update files/en-us/web/api/btoa/index.md

Co-authored-by: rubiesonthesky <[email protected]>

* Update files/en-us/web/api/btoa/index.md

Co-authored-by: rubiesonthesky <[email protected]>

* Update files/en-us/web/api/canvas_api/tutorial/using_images/index.md

Co-authored-by: rubiesonthesky <[email protected]>

* Update files/en-us/web/api/canvas_api/manipulating_video_using_canvas/index.md

Co-authored-by: rubiesonthesky <[email protected]>

* Update files/en-us/web/api/element/getclientrects/index.md

Co-authored-by: rubiesonthesky <[email protected]>

* Update files/en-us/web/api/document/images/index.md

Co-authored-by: rubiesonthesky <[email protected]>

* Update files/en-us/web/api/canvasrenderingcontext2d/arcto/index.md

Co-authored-by: rubiesonthesky <[email protected]>

* Update index.md

* Update index.md

* Update index.md

* Update index.md

* Update index.md

* Update index.md

* Update index.md

* Update index.md

* Update index.md

* Revert change (The world is not ready)

* Use for...of for CSSRuleList

* Update index.md

* Update index.md

* Update index.md

* Update index.md

Co-authored-by: rubiesonthesky <[email protected]>
Co-authored-by: Joshua Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Content:WebAPI Web API docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants