Rework health statistics exercise

[fw-immunant]

No description provided.

[djmitche]

Looking good, thanks!

And, good call on weight -> height ❤️

[djmitche]

Is this intentionally different from HealthReport::visit_count?

[fw-immunant]

Yes--a patient has visited the doctor a given number of times, but a health report corresponds to a particular visit whose count it carries. Maybe visit_index could be used, but I thought that might suggest zero-indexing or that there's a vector of visits somewhere.

[djmitche]

The plural ("visits") suggests a count. If it's not zero-indexed, what is it -- a datestamp?

[fw-immunant]

It's probably least confusing if we just make these variable names match. I'll update them to both be visit_count.

[mgeisler]

@domenukk, see above for your question about usize vs u32.

[michaellee8]

Is there a suggested solution on this exercise? Turns out I need to implement some not so neat approach to solve the Option<(u32, u32)> - (u32, u32) -> Option<(i32, i32)> problem https://gist.github.com/michaellee8/52a8a508f1dfa474b5b88675f23838dc. Is this the best approach one can get?

[djmitche]

There isn't a solution in the repo, but perhaps you'd like to add one?

This is one of a few questions in the exercise that always generates some interesting conversation that is, I think, instructive for students. A few ideas that have come up in these discussions:

• Use i32 everywhere, which covers the range of conceivable blood pressures and differences between them
• When calculating the difference, try_into can get unweildy. Using as is shorter and any undesirable wrapping can be avoided with some explicit checks.
• Define BloodPressure and BloodPressureDifference types, with Sub implemented on BloodPressure returning a BloodPressureDifference. These types could constain their values to reasonable ranges and avoid the need for checks before subtracting.

This is an illustration of Rust's flexibility to meet the situation: as would be fine in a quick script, while custom types with stringent bounds checks probably make sense in a blood-pressure-monitoring application that will be seeking FDA approval.

[fw-immunant]

I would probably do something like this:

let blood_pressure_change = self.last_blood_pressure.map(|last| {
    let meas = measurements.blood_pressure;
    (meas.0 as i32 - last.0 as i32 , meas.1 as i32  - last.1 as i32)
});

However, this does handle overflow by implicitly panicking (in debug) wrapping (in release mode). I don't think there's a very sane behavior available for overflow here: any blood pressure greater than a few hundred is not a realistically possible measurement (much less medically possible) and should rather be caught at the construction of Measurements. This raises the design question of which errors to handle where (at measurement creation or report creation) and I think is a good way to inspire discussion about similar design questions for other Rust codebases.

[michaellee8]

There isn't a solution in the repo, but perhaps you'd like to add one?

This is one of a few questions in the exercise that always generates some interesting conversation that is, I think, instructive for students. A few ideas that have come up in these discussions:

• Use i32 everywhere, which covers the range of conceivable blood pressures and differences between them
• When calculating the difference, try_into can get unweildy. Using as is shorter and any undesirable wrapping can be avoided with some explicit checks.
• Define BloodPressure and BloodPressureDifference types, with Sub implemented on BloodPressure returning a BloodPressureDifference. These types could constain their values to reasonable ranges and avoid the need for checks before subtracting.

This is an illustration of Rust's flexibility to meet the situation: as would be fine in a quick script, while custom types with stringent bounds checks probably make sense in a blood-pressure-monitoring application that will be seeking FDA approval.

I believe that it would have made much more sense to just use i32 since there are no way to have blood pressure exceeding the i32 range. I also agree thst a BloodPressure type would make sense in production code but given the way that the tests are written correctly a solution that uses a custom BloodPressure type will not pass the test. It leads to me having to write custom method to do the subtraction work since it is not possible to implement methods on built-in types like tuples.

[djmitche]

When I teach the course, I encourage students to not feel boxed-in by the existing code, any more than they would be boxed-in by existing code when writing a pull request. But, that is less clear from the contents of this repo.