Correctly use Range from rand #291
Conversation
Codecov Report
@@ Coverage Diff @@
## master #291 +/- ##
==========================================
+ Coverage 87.24% 87.24% +<.01%
==========================================
Files 114 114
Lines 11923 11924 +1
==========================================
+ Hits 10402 10403 +1
Misses 1521 1521
Continue to review full report at Codecov.
|
| @@ -212,10 +213,11 @@ where | |||
|
|
|||
| /// creates random query_id, validates against all active queries | |||
| fn next_random_query_id(&self) -> Async<u16> { | |||
| let between = Range::new(0, u16::max_value()); | |||
There was a problem hiding this comment.
This is still off by one, right? The upper bound is exclusive.
There was a problem hiding this comment.
I read this from their docs: All the primitive integer types satisfy this property, and the float types normally satisfy it, but rounding may mean high can occur: https://docs.rs/rand/0.3.18/rand/distributions/range/struct.Range.html
I took that to mean that the top of the Range is the max int, am I reading that incorrectly?
There was a problem hiding this comment.
The docs confuse me a bit. In one context they discuss primitive integer types satisfying the constraint that it is not inclusive, but then mention but this may be very difficult. And then the next sentence discussing potential rounding up to include the max value. If you know better, I'll change it to +1
Although, going to +1 will require switching up to u32 and then a downcast. Which would be a minor annoyance.
There was a problem hiding this comment.
Ok, I reviewed the src for the Rand. It looks like there is no way for max with u16 to be max. I'll fix this. Thanks for the review.
fixes #253