Define ceiled division, rounding to nearest multiple #16
Conversation
src/lib.rs
Outdated
| /// assert_eq!(23.round_up_to(&8), 24); | ||
| /// ~~~ | ||
| #[inline] | ||
| fn round_up_to(&self, other: &Self) -> Self where Self: traits::NumRef { |
There was a problem hiding this comment.
I like the functionality, but the name seems lacking. I think it could use "multiple" in the name -- maybe next_multiple? Somewhat like the integers' next_power_of_two, which searches >= self.
How should negative self and/or other behave?
(Don't just tell me what it currently does -- think about what it should do first.)
I think I'd rather just clone() instead of introducing NumRef here. Types like BigInt can override with a non-cloning implementation later.
There was a problem hiding this comment.
I think it could use "multiple" in the name -- maybe
next_multiple?
I now called them next_multiple_of and next_multiple_back_of (for consistency with Iterator vs DoubleEndedIterator).
How should negative self and/or other behave?
I'd think it ought to go the other way:
self.next_multiple_of(other) = self.next_multiple_back_of(other.neg())
The new definition fulfills this criterion. I added some tests too.
I think I'd rather just
clone()instead of introducing NumRef here
Done
There was a problem hiding this comment.
I don't care for next_multiple_back_of -- "back" makes more sense for iterators than for numbers. I guess you can thing of it like range iterators, but it still feels clumsy. How about previous_multiple_of or prev_multiple_of?
Please test with negative self values. We should explain in words too, that rounding up/down is reversed with negative other.
There was a problem hiding this comment.
I think previous_multiple_of is too long, and the abbreviation prev could be ambiguous — preview is the clash which readily comes to mind, but i'm not sure how relevant it is here. How about past_multiple_of?
There was a problem hiding this comment.
I like prev better than past. I don't see any precedent in the std API supporting that, but there's a lot of use of prev_ meaning previous throughout rust's implementation. So I think that's well understood, especially in context with next_.
src/lib.rs
Outdated
| /// assert_eq!(23.round_down_to(&8), 16); | ||
| /// ~~~ | ||
| #[inline] | ||
| fn round_down_to(&self, other: &Self) -> Self where Self: traits::NumRef { |
There was a problem hiding this comment.
Same design questions as round_up_to.
|
ping |
|
Done, PTAL |
|
Looks good, I just formatted it. bors r+ |
16: Define ceiled division, rounding to nearest multiple r=cuviper a=strake I often find `round_up_to` in particular useful; `round_down_to` is also defined for completeness. `div_ceil` is also defined as `round_up_to` is defined in terms of it. Co-authored-by: M Farkas-Dyck <strake888@gmail.com> Co-authored-by: Josh Stone <cuviper@gmail.com>
Build succeeded |
I often find
round_up_toin particular useful;round_down_tois also defined for completeness.div_ceilis also defined asround_up_tois defined in terms of it.