Course feedback from students

[marshallpierce]

Feedback from day 1:

• exercise 10.6 (elevator) wasn't very clear as to what the desired end state was -- not confusion over enum syntax per se, but more like what the success criteria was.
• The exercise requires #[derive(Debug), so maybe we should provide an empty Direction with that attribute already there, and have them fill in that type and add more? fixed in Clarify completion condition for elevator exercise #1574

[marshallpierce]

Day 2 and 3 feedback

12.3 Expression evaluation

• Uses Box before we introduce it, so * needed for eval(*left) is nonobvious
• The matches in eval is a bit awkward, and doesn't make use of the let control flow in the section. It uses destructuring, but not in a particularly enlightening way
• unused code warnings led to questions, so this sample should likely also have the allow(unused) snippet applied
• Discussion about error handling and how it's different from idiomatic error handling led to some comments that they'd rather just use real Result, even though it's not been covered yet

13.5 GUI

• More time spent on formatting text than on traits (GUI Exercise is more about text formatting than trait objects #1617)

19.8

• as_dependency semantics weren't clear about whether it was for the package itself or one of its dependencies

20.1 Box

• Cons is confusing to the non-Lisp folks

20.3 Binary tree

• The BinaryTreeNode / BinaryTree terminology was confusing due to a node itself having left and right trees. Perhaps a type alias for the wrapped Option<...> type would address this.
• Same concerns about unused code warnings as above
• C++ folks found it odd that has() didn't take a reference

22.4 Health statistics

• Would have been helpful to have comments on struct fields
• u32 to i32 conversions are bad (note that the solution doesn't follow my suggestion to avoid as). Could use u16 to allow .into() conversion to i32, and still cover all conceivable blood pressures.

23.3 Lifetime annotations

• First sentence seems backwards: values/references must outlive the lifetimes that refer to them, not the other way around
• Sample not editable, probably accidentally
• Could use more clarity on why or when people would want to use lifetimes -- when is it worth going to the trouble of not Cloning, etc.

23.6 Protobuf

• Relies on error handling, ?, etc, but nobody was put off by that, lending more weight to earlier comments to use real Result instead of similar, simpler types
• More about error handling that lifetimes or slices -- one of the strongest consensus for any of the feedback here
• Some trouble figuring out how to get started on the problem
• I used the following helper in my solution, and people liked it enough to suggest having it in the starting code for the exercise

  • fn maybe_split_at(slice: &[u8], len: usize) -> Result<(&[u8], &[u8]), Error> {
         slice
             .get(..len)
             .map(|prefix| (prefix, &slice[len..]))
             .ok_or(Error::UnexpectedEOF)
     }

Misc

• Running tests vs running main was confusing -- would be good if we could pick one and stick with it, or when that's not appropriate, make it clear when one or the other is to be used, perhaps via main() { panic!("Please run the tests") } or the like.

[edited to add checkboxes --@djmitche]

[djmitche]

Breaking out a bit of this into other issues:

• For tests vs. main, and allow(unused), see newly-filed Exercise consistency: tests or main? and allow(unused) #1581.
• For Health Statistcs, see "Borrowing" exercise isn't very good #1513.

[djmitche]

C++ folks found it odd that has() didn't take a reference

I think this is actually an interesting learning opportunity - the data structure is built around T: Copy, so there's no need to take a reference here (and in fact, for Copy types like i32 and &str it's less ergonomic to do so).

[marshallpierce]

Yeah, they just thought the Copy restriction was odd, and with AsRef, it can be ergonomic either way

[djmitche]

A few of these became issues of their own, but everything here is handled one way or another.