RE: [boost] Bug in boost/rational.hpp

From: Paul Moore (gustav_at_[hidden])
Date: 2000-11-19 16:57:19

From: ebo_at_[hidden] [mailto:ebo_at_[hidden]]
> Description and reproducion:
>
>   The devision of a rational by "0" does not throw an exception but
>   returns a 0 denominator in the result.  If you trace through the
>   rational code example you will find that the exception is actually
>   never generated by the test code.
>
> Effected code segment:
>
>   boost/rational.hpp: 195-206
>
> Proposed Fix:
>
>   Add a bad_rational test (denominator == 0) test and throw an
>   exception.
>
>   template <typename IntType>
>   rational<IntType>& rational<IntType>::operator/= (const
> rational<IntType>& r)
>   {
>     // Avoid overflow and preserve normalization
>     IntType gcd1 = gcd<IntType>(num, r.num);
>     IntType gcd2 = gcd<IntType>(r.den, den);
>     num = (num/gcd1) * (r.den/gcd2);
>     den = (den/gcd2) * (r.num/gcd1);
> >>>    // validity check
> >>>    if (0 == den) throw bad_rational();
>     return *this;
>   }

Good point. I missed this case. And thanks for the clear and detailed bug
report.

I'd rather add a check as

template <typename IntType>
rational<IntType>& rational<IntType>::operator/= (const rational<IntType>&
r)
{
    // Trap division by zero
    if (r.num == 0) throw bad_rational();
    // Avoid overflow and preserve normalization
    IntType gcd1 = gcd<IntType>(num, r.num);
    IntType gcd2 = gcd<IntType>(r.den, den);
    num = (num/gcd1) * (r.den/gcd2);
    den = (den/gcd2) * (r.num/gcd1);
    return *this;
}

I.e., test at the top, on the pinciple of avoiding unnecessary work. Why
calculate two GCDs when you won't use them?

A question - should this throw bad_rational() or is there a standard divide
by zero exception? (Couldn't find one in a brief glance in the standard...)

Beman, could you add this or would you like me to send a complete
replacement rational.hpp?

> Notes:
>
>   as an aside, years ago I learned a little programming trick which I
>   used above.  By placing the literal on the left hand side of the
>   expression I use compilers parser to catch certain types of typos.
>   In this way if I inadvertently type a "=" instead of a "==" the
>   compiler will see it as a syntax error.  i.e. a "if (0=den)"
>   produces a syntax error where as "if (den=0)" is valid but not what
>   I wanted.  Just an aside...

I've seen this before, and the rationale is good. But to be honest, no
matter how good the reason, I simply can't stand the look of the code. 0=den
just reads completely wrongly to me...

Paul.