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

Reorder material on first two days #913

Merged
merged 5 commits into from
Aug 25, 2023
Merged

Conversation

fw-immunant
Copy link
Collaborator

@fw-immunant fw-immunant commented Jul 6, 2023

This will require some discussion and some changes to exercises, but hopefully should be useful as a PR at least to get something concrete to discuss.

Basically, my thoughts are:

  • Morning of day 1 still introduces the language and its high-level goals/value proposition, and starts with the built-in data types Rust provides, and how you define a function. I'm thinking the implicit-conversions exercise could be expanded to cover more about overflow/underflow/divide-by-zero behavior and maybe checked operations, to emphasize what Rust does differently than other languages (in particular C and C++) here. The arrays and for loops exercise can lead in to the next topic, which will (after reorg) be control flow.
  • Afternoon of day 1 gets a frontloading of the basic control flow structures in Rust but not the more exotic ones. This lets us introduce enums prior to match, motivating match. Then we get into patterns, which we can give a mix of match/if let/while let/let ... else. The exercises for day 1 afternoon will be the Luhn algorithm (where we can match on digits and enums such as Option and practice working with variables) and some other pattern-matching exercise, perhaps one where we dig deep into a nested data structure with matching. I sometimes do this synchronously with students while going over the pattern matching slides at present, but it could be split out into a proper exercise.
  • Morning of day 2 still has discussion of memory management, but we take out the "Points and Polygons" in favor of something simpler that still works out structs/ownership/method receivers. I'm not yet sure what that looks like, but the expanded "Health Statistics" exercise (Rework health statistics exercise #909) is partway there. Implementing an iterator is a great exercise but seems like too much alongside all the rest of what Points+Polygons does, so maybe we could strip it down to just defining an iterator.
  • This does deflate day 2 afternoon a fair bit, so I think some more shuffling or just moving the day divisions might help to keep things balanced. As mentioned, this is a WIP, and mostly to inspire discussion at this point.

Feedback welcome!

Fixes #510.

@mgeisler
Copy link
Collaborator

mgeisler commented Jul 7, 2023

Thanks @fw-immunant!

When discussing with @gribozavr and @scentini, we talked about moving more of the difficult stuff to the morning slots: the thinking was that people would be more energized there compared to an afternoon slot. So moving almost all of the syntax slides to Day 1 afternoon seems good!

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.

Left some minor comments here, but I suspect that PRs aren't the best medium for discussing a reorganization of the course. I'm not sure what is, though!

src/SUMMARY.md Outdated Show resolved Hide resolved
src/SUMMARY.md Show resolved Hide resolved
Copy link
Collaborator

@mgeisler mgeisler left a comment

Choose a reason for hiding this comment

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

I paged through the course locally and I like it! Some small notes:

  • Please update the small course overviews on

    • running-the-course/course-structure.md
    • welcome-day-1.md, and
    • welcome-day-2.md
  • Please create a control-flow/novel.md file for the new chapter.

  • Can we add a small pattern matching exercise which asks students to evaluate an expression tree using a recursive function? Something like this. All the boxing might be rather confusing, though?

    Alternatively, we should make the link a placeholder like in my other comment.

  • Please remember to update the redirect table in book.toml if you move any file (doesn't seem to be the case here)

src/SUMMARY.md Outdated Show resolved Hide resolved
src/SUMMARY.md Show resolved Hide resolved
src/SUMMARY.md Outdated Show resolved Hide resolved
src/SUMMARY.md Outdated Show resolved Hide resolved
@mgeisler
Copy link
Collaborator

Hey @fw-immunant, I like this a lot! I would like you to already think about how we can remove the "Basic Syntax" section (or at least rework it). It doesn't make so much sense after this PR since we cover most of it again right after the break:

image

So we should have a PR to fix that right after merging this one 😄

@fw-immunant fw-immunant force-pushed the fw/reorder branch 3 times, most recently from 9892d56 to 68e34a8 Compare August 10, 2023 20:11
@fw-immunant fw-immunant marked this pull request as ready for review August 10, 2023 20:11
@fw-immunant fw-immunant force-pushed the fw/reorder branch 5 times, most recently from fdd33ca to 4cbec64 Compare August 10, 2023 20:24
@fw-immunant
Copy link
Collaborator Author

From CI:

Run mdbook build
2023-08-10 20:24:56 [INFO] (mdbook::book): Book building has started
2023-08-10 20:24:56 [INFO] (mdbook::book): Running the exerciser backend
2023-08-10 20:24:56 [INFO] (mdbook::renderer): Invoking the "exerciser" renderer
2023-08-10 20:24:56 [INFO] (mdbook::book): Running the html backend
Error: -10 20:24:58 [ERROR] (mdbook::utils): Error: Rendering failed
Error: -10 20:24:58 [ERROR] (mdbook::utils): 	Caused By: Unable to emit redirects
Error: -10 20:24:58 [ERROR] (mdbook::utils): 	Caused By: Not redirecting "/home/runner/work/comprehensive-rust/comprehensive-rust/book/html/exercises/day-1/book-library.html" to "../day-2/book-library.html" because it already exists. Are you sure it needs to be redirected?
Error: Process completed with exit code 101.

But this PR does remove day-1/book-library.md, so presumably we need to add a redirect there. Am I missing something?

src/SUMMARY.md Outdated Show resolved Hide resolved
@mgeisler
Copy link
Collaborator

Hi @fw-immunant, I think this just needs a little rebase on top of current main and then it should be good to go 😄

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.

Let's get this landed :)

This has to be an empty link until we add the page — mdbook creates a grayed-out heading in the sidebar for empty links, but it creates an empty page for actual links:
Copy link
Collaborator

@mgeisler mgeisler left a comment

Choose a reason for hiding this comment

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

Thanks for much, I'm excited to see this in action!

@mgeisler mgeisler changed the title WIP: rework ordering Reorder material on first two days Aug 24, 2023
@mgeisler
Copy link
Collaborator

@fw-immunant please merge this at your convenience! 🚀

Let me push a small commit to your branch with a page for the novel control flow.

@mgeisler
Copy link
Collaborator

I found a forgotten rename and pushed a fix for that too. It looks good from my side now — perhaps skim over it and then merge the PR!

@mgeisler mgeisler merged commit d3a9037 into google:main Aug 25, 2023
30 checks passed
mgeisler added a commit that referenced this pull request Aug 28, 2023
yohcop pushed a commit to yohcop/comprehensive-rust that referenced this pull request Sep 12, 2023
- Morning of Day 1 still introduces the language and its high-level
goals/value proposition, and starts with the built-in data types Rust
provides, and how you define a function. 
- Afternoon of Day 1 gets a front loading of the basic control flow
structures in Rust but not the more exotic ones.
- The exercises for day 1 afternoon will be the Luhn algorithm (where we
can match on digits and enums such as `Option`.
- Morning of day 2 still has discussion of memory management.

Fixes google#510.

---------

Co-authored-by: Martin Geisler <mgeisler@google.com>
@mgeisler mgeisler mentioned this pull request Oct 31, 2023
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.

Methods used in exercise before they are introduced
3 participants