Skip to content

Commit

Permalink
Merge #51
Browse files Browse the repository at this point in the history
51: Fix #49 r=cuviper a=maxbla

fixed issue #49 
added a test case for it

Side note:
`cmp` is a bit scary. Do we know what it's amortized runtime is? It seems to me that the worst case (having to compare the reciprocals many times) could  be pretty bad. Is there any way to have a separate `impl` for `T: Clone + Integer + CheckedMul`? As the comments note, CheckedMul would make the implementation faster. I messed around, but I can't find a way to have two implementations -- one for checked and one for no checked. I found an [this](rust-lang/rfcs#586) RFC for negative trait bounds though (spoiler: negative trait bounds are not happening soon).

Co-authored-by: Max Blachman <[email protected]>
  • Loading branch information
bors[bot] and maxbla committed Jul 18, 2019
2 parents c53f2d7 + f38ff9c commit d28b12f
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,9 @@ impl<T: Clone + Integer> Ord for Ratio<T> {

// With equal numerators, the denominators can be inversely compared
if self.numer == other.numer {
if self.numer.is_zero() {
return cmp::Ordering::Equal;
}
let ord = self.denom.cmp(&other.denom);
return if self.numer < T::zero() {
ord
Expand Down Expand Up @@ -1473,6 +1476,9 @@ mod test {

assert!(_0 >= _0 && _1 >= _1);
assert!(_1 >= _0 && !(_0 >= _1));

let _0_2: Rational = Ratio::new_raw(0, 2);
assert_eq!(_0, _0_2);
}

#[test]
Expand Down

0 comments on commit d28b12f

Please sign in to comment.