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

add iterators exercise #161

Merged
merged 20 commits into from
Jan 21, 2025
Merged

add iterators exercise #161

merged 20 commits into from
Jan 21, 2025

Conversation

miguelraz
Copy link
Contributor

No description provided.

@miguelraz miguelraz changed the title add exercise-{solutions|templates}/iterators add iterators exercise Jan 14, 2025
Copy link

cloudflare-workers-and-pages bot commented Jan 14, 2025

Deploying ferrous-systems-rust-exercises with  Cloudflare Pages  Cloudflare Pages

Latest commit: a014833
Status: ✅  Deploy successful!
Preview URL: https://798191d7.ferrous-systems-rust-exercises.pages.dev
Branch Preview URL: https://iterators-exercise.ferrous-systems-rust-exercises.pages.dev

View logs

@miguelraz miguelraz marked this pull request as ready for review January 15, 2025 05:45
@miguelraz
Copy link
Contributor Author

I still don't know why the code exits with a 1.


- Take the template in [exercise-templates/iterators](../../exercise-templates/iterators/) as a starting point.
- Replace the first `todo!` item with [reader.lines()]() and continue "chaining" the iterators until you've calculated the desired result.
- Run the code with `cargo run --bin iterators1` when inside the `exercise-templates` directory
Copy link
Member

Choose a reason for hiding this comment

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

While for embedded training we expect the group to clone exercises repo and work within it, for our intro exercises we actually prompt them to make a separate new rust project for each of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Rephrased this section. Will wait on another review until I resolve.


let result = reader.lines()
.map(|l| l.unwrap())
.filter_map(|s| s.parse().ok())
Copy link
Member

Choose a reason for hiding this comment

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

While I appreciate a gradual progression on the solution, this particular sequence of steps (Step 2 to Step 6) goes against of what we teach people on trainings. They don't "unwarp() now, refactor later". Instead as soon as trainee sees a Result, they will think how to properly handle it. The final solution won't have any unwrap / expect calls in code. So I would give a different hint early: about the fact that in this problem we skip over "bad" lines, and then use this fact to introduce filter_map as an important method.

Copy link
Member

@listochkin listochkin left a comment

Choose a reason for hiding this comment

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

Added some suggestions here and there, but very happy with the overall structure: tons of help offered, and the overall flow feels really nice.

miguelraz and others added 6 commits January 20, 2025 07:42
Co-authored-by: Andrei Listochkin (Андрей Листочкин) <andrei.listochkin@ferrous-systems.com>
Co-authored-by: Andrei Listochkin (Андрей Листочкин) <andrei.listochkin@ferrous-systems.com>
@miguelraz miguelraz requested a review from listochkin January 20, 2025 14:03
miguelraz and others added 4 commits January 21, 2025 08:07
Co-authored-by: Andrei Listochkin (Андрей Листочкин) <andrei.listochkin@ferrous-systems.com>
Co-authored-by: Andrei Listochkin (Андрей Листочкин) <andrei.listochkin@ferrous-systems.com>
@listochkin
Copy link
Member

Don't forget to add it to ToC!

@listochkin listochkin merged commit 9854185 into main Jan 21, 2025
3 of 5 checks passed
@miguelraz miguelraz deleted the iterators-exercise branch January 21, 2025 14:44
@jonathanpallant
Copy link
Member

please don't merge things unless CI is green

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