BigRational(whole, numerator, denominator) is confusing with negative parts

Sep 16, 2010 at 1:12 AM

BigRational(whole, numerator, denominator) sets m_denominator = denominator and m_numerator = (whole * denominator) + numerator.  That math makes sense, but it gives a non-intuitive result when negative portions are involved.  For example, BigRational(-3, 1, 2) calculates m_numerator as (-3*2)+1 = -5, which gives a rational value of -5/2.  Since the constructor looks like it takes the three parts of a mixed fraction, I would have expected it to return the equivalent of -3 1/2, which is -7/2.

This constructor appears to take the parts of a mixed fraction, but it doesn't follow the arithmetic rules for a mixed fraction.  In a mixed fraction, -a b/c is interpreted as -1 * (a + b/c), which is -a - b/c.  So in a mixed fraction (which is just shorthand for the longer algebraic notation), the sign of the whole determines the sign of the overall value, and signs are unused on the fraction's numerator and denominator.

The fact that BigRational's three arg constructor allows each part to retain its sign causes BigRational's confusing behavior.  It should probably only use the sign from the whole portion, and then require the numerator to be non-negative and the denominator to be positive.  Or change the member calculations to:

m_denominator = BigInteger.Abs(denominator);
m_numerator = (whole * m_denominator) + (whole.Sign < 0 ? -1 : 1) * BigInteger.Abs(numerator);

I just wanted to point this out as the one quirky design issue I ran into with BigRational.  I expected the three arg constructor to use mixed fraction semantics, and the fact that it didn't work that way led to a subtle bug with negative mixed fractions in my Silverlight RPN Calculator.

I'm aware that the three arg constructor works consistently with how GetWholePart and GetFractionPart behave because the math is simple.  I'm just saying from a design point of view that this constructor behaves non-intuitively because of what mixed fraction notation means (-a b/c is - a - b/c).

Oct 27, 2010 at 6:10 PM

I agree, this constructor is confusing. I don't know of anyone who would think of a rational number of "-3 1/2" (that is, "-3.5") as "-3 + 1/2" (or "-3 + 0.5"). The ctor:

BigRational(whole, numerator, denominator)

makes me think I'm putting in the terms as @bmenees does, as "-3 1/2".