jetbrains-academy / rustlings-course

Small exercises to get you used to reading and writing Rust code!
https://github.com/rust-lang/rustlings/releases/tag/1.2.2
MIT License
149 stars 33 forks source link

suggestion: Common Collections / Options / Use Option #291

Closed Tsovak closed 10 months ago

Tsovak commented 10 months ago

the task Common Collections / Options / Use Option description:

We have the maybe_ice_cream function that returns how much ice cream there is left in the fridge. If it's before 10PM, there's 5 pieces left. At 10PM, someone eats them all, so there'll be no more left :(

The suggestion is to have 2 different functions with match and the current implementation. Does it make sense?

the current implementation is:

pub fn maybe_ice_cream(time_of_day: u16) -> Option<u16> {
    // We use the 24-hour system here, so 10PM is a value of 22 and 12AM is a value of 0
    // The Option output should gracefully handle cases where time_of_day > 23.
    if time_of_day < 22 {
        Some(5)
    } else if time_of_day < 24 {
        Some(0)
    } else {
        None
    }
}

one more implementation is:

    match time_of_day {
        0..=22 => Some(5),
        22..=24 => Some(0),
        _ => None,
    }
image
kochaika commented 10 months ago

Hi! Thanks for the feedback!

I don't really understand your idea.

Implementation with if-else is the correct solution. But not the only one. There can be many correct solutions. But to the user when viewing the "correct solution" is displayed solution with if-else.

Your solution using match is almost correct too. The test fails because the interval 0...=22 includes 22, and the task expects strictly less (...before 10PM).

Tests do not check the code match, but how the program works. So it doesn't matter how exactly the user solved the task. And as a "correct answer" that the user can look up, it is enough to specify one of them (it doesn't affect the check). The authors of the course chose the if-else

Does it become clearer after my answer? Is this issue still relevant?

Tsovak commented 10 months ago

yes, it does