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

Mention both static and dynamic dispatch in the introduction to traits. #376

Closed
wants to merge 1 commit into from

Conversation

gendx
Copy link
Collaborator

@gendx gendx commented Feb 9, 2023

I find it weird to start the introduction to traits with dynamic dispatch only, which comes at a runtime cost. Not mentioning any static dispatch might give a wrong impression that traits are not a zero-cost abstraction.

I anticipate the question anyway, so better have the answer on the slide already.

@@ -25,7 +25,19 @@ impl Greet for Cat {
}
}

// Generic function (more in the next chapter).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you try enabling the "aspect-ratio helper" preprocessor here: https://github.com/google/comprehensive-rust/blob/main/book.toml#L24. I think you'll quickly see that there is not enough space on the page for such a long code example.

@mgeisler
Copy link
Collaborator

mgeisler commented Feb 9, 2023

I find it weird to start the introduction to traits with dynamic dispatch only, which comes at a runtime cost. Not mentioning any static dispatch might give a wrong impression that traits are not a zero-cost abstraction.

I don't disagree 😄 We will need to reorder the slides a bit and make the whole things more streamlined.

However, I'm afraid of putting more stuff onto the pages. When I did the course last, I was constantly running out of time and as a result, the course felt rushed to me. So I think we need to go the other way and trim away unnecessary stuff (or rebalance the pages some other way).

@gendx
Copy link
Collaborator Author

gendx commented Feb 14, 2023

I don't disagree 😄 We will need to reorder the slides a bit and make the whole things more streamlined.

However, I'm afraid of putting more stuff onto the pages. When I did the course last, I was constantly running out of time and as a result, the course felt rushed to me. So I think we need to go the other way and trim away unnecessary stuff (or rebalance the pages some other way).

So after teaching the first 3 days, the session on traits and generics was by far the most dense, because it's introducing a lot of new concepts (traits, generics, dynamic/trait objects), of new syntax (trait bounds, where clauses, impl Trait, dyn Trait, closures), as well as important traits from the standard library.

It would IMO make sense to prioritize what to teach, and focus on that first.

In terms of streamlining this session, I'd see the following fundamentals to focus on, because they are the foundation for everything else:

  1. Traits,
  2. Generics types + methods,
  3. Trait bounds + where clauses (but not the impl Trait syntactic sugar),
  4. Important traits for the standard library -- after the syntax for trait bounds, because they use them (the first thing one sees opening the Iterator documentation is a bunch of where clauses).

I'd leave the other topics to a lower priority for the following reasons:

  • Closures: The concepts of Fn, FnMut and FnOnce are non-trivial and would deserve more illustration to explain the difference. I ended up editing the slide a lot to show mutable closures (that capture variables), syntax for closures with multiple parameters (vs. a single tuple parameter), etc., but not having snippets pre-prepared made me spend more time on it. It'd be useful to expand that into multiple slides for (1) the syntax and (2) the differences between all the function traits. That closures are move-able could also deserve some illustration.
  • Monomorphization: It's good to know what happens under the hood, but it adds more vocabulary without unlocking new language syntax/features. So if running out of time I would drop that.
  • impl Trait: That's a nice syntactic sugar, but this session already introduced so many new pieces of syntax that it can drown the students in all the many ways of doing the same thing.
  • Trait objects and dynamic dispatch: There's a lot to be said about the memory layout (fat pointers, virtual tables). However:
    1. I found it weird to introduce dyn Trait from the introduction slide, because that led to a digression on what this syntax meant, what the memory layout might look like, which eventually would be covered later in the dedicated slide anyway. So if anything I wouldn't mention the dyn syntax until the last slide of this session.
    2. I find generics + trait bounds more important than trait objects: they are used everywhere in the standard library, they are generally more efficient -- apart from generated binary size due to monomorphization (although inlining optimizations unlocked by static polymorphism may counter-balance monomorphization, as is often the case with loops and iterators).
    3. Trait objects are "less novel" in Rust vs. other languages like C++/Java, because they can easily map to OOP concepts (btw the slide on trait objects could make an explicit mention to OOP). On the other hand, I find generics with trait bounds and precise where clauses more unique to Rust (vs. template metaprogramming in C++ for example).
    4. In that sense, I found the GUI library exercise not as interesting as it could be: all the code maps 1:1 with OOP concepts like inheritance, without exploring Rust's unique syntax for trait bounds.

@mgeisler
Copy link
Collaborator

So after teaching the first 3 days, the session on traits and generics was by far the most dense, because it's introducing a lot of new concepts

You are quite right! Thanks for the ideas about restructuring this — I completely agree that we should turn this into something better. I've run into many of the same problems with the flow as you mention (cc @gribozavr).

@gendx gendx marked this pull request as draft February 15, 2023 08:28
@mgeisler
Copy link
Collaborator

Hey @gendx!

The material has been shuffled around a bit since you wrote this PR. I don't know if it still applies?

I think we should close this and the try to fix this in the reorg done by @djmitche in #1073.

@mgeisler mgeisler closed this Sep 16, 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.

2 participants