Re: Rational bug

From: vesa_karvonen (vesa_karvonen_at_[hidden])
Date: 2002-02-09 16:56:59

--- In boost_at_y..., "David Abrahams" <david.abrahams_at_r...> wrote:
> The definition of operator /= for class template
> rational does not handle self divisions (as in r /=
> r;) correctly:

Whoever fixes this should make sure that a regression test is added 
to test self division. These kind of things are likely to be broken 
by "optimizations" or "simplifications" accidentaly.

> Another solution would be to add:
> 
> if(this == &r)
> num = den = IntType(1);
> return *this;
> }

The above solution is likely to be slower when the IntType is a built-
in type. I would avoid it. The compiler can't, in general, optimize 
away the conditional test. Self divisions are also likely to be so 
infrequent (proof: it took a long time to notice the bug) that 
optimizing for them is likely to be useless.

> This code should be changed to:
> 
> IntType rnum = r.num;
> num = (num/gcd1) * (r.den/gcd2);
> den = (den/gcd2) * (rnum/gcd1);

The above is likely to improve the efficiency of the code when 
IntType is a built-in type. It effectively tells the compiler that it 
can cache rnum in a register.