-
Notifications
You must be signed in to change notification settings - Fork 22.6k
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
JS Learning Area updates: looping code #10645
Conversation
Preview URLs
FlawsNone! 🎉 External URLsURL: No new external URLs URL: No new external URLs (this comment was updated 2021-11-27 05:25:39.480082) |
@@ -297,15 +428,17 @@ btn.addEventListener('click', function() { | |||
let searchName = input.value.toLowerCase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const ?
Is @teoli2003 on holiday? I'm hesitant to review if you can get someone with more expertise. |
It's up to you, I'm happy for you to review this if you want, I'm sure you have plenty of expertise. Also this is introductory so reviewing for clarity and consistency are particularly important, and there shouldn't be anything technically very complex here. |
2. Assign it to the variable `cat` and then run the code between the curly brackets `{}`. | ||
3. Get the next item, and repeat (2) until you've reached the end of the collection. | ||
|
||
Note that we have to use `let` for the `cat` variable, because we're assigning it to a new item each time we go round the loop. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this true? My understanding was that you could use const, let, or var because the scope of cat (for let and const) is within the loop. You can't use const if you then want to modify cat in the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If so it might be worth modifying item 2 above. This kind of implies the variable is created once and reassigned to a new value on each loop. I think that is true for var, but not for let.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! Apparently I don't understand const either!
@wbamberg Really good. I am happy to merge as is except for my comment above #10645 (comment) In terms of other things I noticed that we might want to look at
Last of all, I did inject some layout changes which you can ignore. I just do this because shorter lines are easier to review in github diffs. I tend to break on sentences. |
Loops, loops, loops. As well as being associated with [popular breakfast cereals](https://en.wikipedia.org/wiki/Froot_Loops), [roller coasters](https://en.wikipedia.org/wiki/Vertical_loop), and [musical production](<https://en.wikipedia.org/wiki/Loop_(music)>), they are also a critical concept in programming. Programming loops are all to do with doing the same thing over and over again, which is termed **iteration** in programming speak. | ||
Loops, loops, loops. | ||
As well as being associated with [popular breakfast cereals](https://en.wikipedia.org/wiki/Froot_Loops), [roller coasters](https://en.wikipedia.org/wiki/Vertical_loop), and [musical production](<https://en.wikipedia.org/wiki/Loop_(music)>), they are also a critical concept in programming. | ||
Programming loops are all to do with doing the same thing over and over again, which is called **iteration** in "programming speak". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI this is an actual minor change.
Yeah, I wasn't sure what to do about that. I couldn't really see how to adapt it to make it more
Yes, good point. I didn't look too closely at those, but while I am here it's worth adding that. |
I would. I'm not sure it adds anything over what is in the "why bother" section. Also someone is going to complain about the farmer being a "he" one day. |
OK, I have:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, heaps better. Particularly the removal of the farmer stuff at the top - that just felt like wasted time.
I was going to make a comment that we should also update "Which loop type should you use?" to mention that you use "do...while` loop if you have to run it once. But not sure that was needed - what is there now captures the flavour/intent.
Part of #10337.
This PR updates the "Looping code" part of the JS Learning Area: https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Building_blocks/Looping_code.
The main thing here is to:
for...of
as the recommended way to iterate through a collectionmap()
andfilter()
for...of
are doing so.It's a bit tricky because we have already introduced
for...of
,map()
, andfilter()
in the Arrays page, but I thought it's going to be easiest for readers just to present it again here.I've also made a tiny update to the https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Building_blocks/Test_your_skills:_Loops page, to reflect updates to the learning area in mdn/learning-area#421.