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

Rework iterator section #2523

Merged
merged 8 commits into from
Dec 17, 2024
Merged

Rework iterator section #2523

merged 8 commits into from
Dec 17, 2024

Conversation

randomPoison
Copy link
Collaborator

This PR attempts to improve clarity and flow in the section on iterators. I don't think the current materials do a good job of motivating iterators or putting them in a context where students can understand them. This PR does the following to try to improve this:

  • Add motivation slide. This slide uses the example of using a C-style for loop to iterate over the contents of an array. This helps contextualize the concept of iteration in a way that most students will understand; Many languages have C-style for loops, so students are likely going to be familiar with the syntax and will have seen for (i = 0; i < len; i++) many times.
  • Replace fibonacci example with slice iter example. This continues the idea from the previous slide, demonstrating how to implement the same concept (iterating over the contents of an array) using Iterator. I also think this example is easier to understand than the fibonacci one, I found the logic there kind of confusing/hard to explain to students. It also demonstrates an iterator with a termination case, whereas the fibonacci example never terminates.
  • Add slide for iterator helper methods. I think splitting helper methods into its own slide gives them a bit more room to breathe. This also allows the previous slide to be more focused on implementing Iterator.
  • Reorder slides to talk about collect before IntoIterator. I think this flows better after the slide on helper methods since this is just calling out a specific helper method.
  • Add speaker notes and reword things for clarity/focus.

Comment on lines +18 to +20
for (int i = 0; i < array_len; i += 1) {
int elem = array[i];
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As I put this PR up it occurs to me that we don't need to use C-style for to demonstrate this concept, we could show the same thing in pure Rust:

let mut i = 0;
while i < array.len() {
    let elem = array[i];
    i += 1;
}

Might still be good to mention C-style for in the speaker notes, since it's still a common point of reference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I kinda still feel like the C version is going to be clearer and more familiar for most students, tho

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@djmitche Do you have an opinion here? I'm leaning towards leaving this as a c-style for for readability and familiarity

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You know what, I'm going to leave it as a C-style for but I'm going to add the rust equivalent in the speaker notes for easy reference. Let me know if you think that's a bad idea for whatever reason, though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the C-style is fine.

- Add motivation slide.
- Replace fibonacci example with slice iter example.
- Add slide for iterator helper methods.
- Reorder slides to talk about `collect` before `IntoIterator`.
- Add additional speaker notes.
@randomPoison randomPoison force-pushed the legare/iterator-rework branch from c162cfd to c66a766 Compare December 16, 2024 23:31
Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

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

This is a great improvement to this segment!

Comment on lines 17 to 18
impl<'a> Iterator for SliceIter<'a> {
type Item = &'a i32;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor detail, but I'd like to get away from the idea that all lifetimes are named 'a -- maybe use 'sl instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about 's?

Copy link
Collaborator

Choose a reason for hiding this comment

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

's is good!

src/iterators/iterator.md Show resolved Hide resolved
src/iterators/helpers.md Outdated Show resolved Hide resolved
src/iterators/helpers.md Outdated Show resolved Hide resolved
src/iterators/helpers.md Outdated Show resolved Hide resolved
randomPoison and others added 5 commits December 17, 2024 14:19
@randomPoison randomPoison merged commit 7f0c591 into main Dec 17, 2024
37 checks passed
@randomPoison randomPoison deleted the legare/iterator-rework branch December 17, 2024 23:59
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.

2 participants