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

Fixed ch03-05-control-flow.md, replaced for with while #3757

Closed
wants to merge 2 commits into from

Conversation

Atabic
Copy link

@Atabic Atabic commented Oct 26, 2023

Hi team,
The section heading was Looping Through a Collection with for
and instead the explanation under it is about while.
Updated a link in ch12 which caused one of the job to fail

@Atabic
Copy link
Author

Atabic commented Oct 26, 2023

Hi @carols10cents . Can you please review this whenever you get time?

@RafaelKr
Copy link
Contributor

Please read that section again. It starts by explaining looping over a collection with the previously introduced while loop. Then it tells you there is a much better way by using for. So this section heading is correct.

@Atabic
Copy link
Author

Atabic commented Oct 27, 2023

@RafaelKr But the section under it explains all about how to loop over a collection using while instead of for.
image

@RafaelKr
Copy link
Contributor

RafaelKr commented Oct 27, 2023

Yes, but it does it to explain why using a while in this case isn't as good as for.

To be fair I read this very section the first time 2 hours ago and also wondered why it is labelled for and starts with a code block about a while loop. But after reading the section, things cleared up for me.

My suggestion is to update the wording of the first sentence instead:

- You can choose to use the while construct to loop over the elements of a collection
+ You could choose to use the while construct to loop over the elements of a collection

So changing can to could. This makes it clearer that while is a way to do it, but for is much better as explained later starting at However, this approach [using a while loop] is error prone; we could cause the program to panic if the index value or test condition is incorrect...

@RafaelKr
Copy link
Contributor

@Atabic I just created #3758 with my suggestion.

@rust-lang rust-lang deleted a comment from hurrywillsonfromuk Mar 27, 2024
@chriskrycho
Copy link
Contributor

Closing in favor of #3758. The heading order here is intentional, but that small wording tweak should help. Thanks!

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

Successfully merging this pull request may close these issues.

3 participants