Return to Answer

In addition to ChrisWue's comments, I'd add:

In addition to ChrisWue's comments, I'd add:

1. Even small computations with your rationals result in integer overflow. For example, consider the following program:

    int main(int argc, char **argv) {
    Rational sixteenth = Rational_Construct(1, 16);
    Rational sum = Rational_Construct(0, 1);
    int i;
    for (i = 0; i < 10; ++i) {
    sum = Rational_Add(sum, sixteenth);
    printf("%d/%d\n", sum.Top, sum.Bottom);
    }
    return 0;
    }
   

Now, because you're using C's fixed-size integers, there are bound to be computations whose results are too large to be represented. But it would be better to raise an error when the result is too large, rather than causing undefined behaviour. And you should at the very least make some effort to ensure that integer overflow doesn't happen in small examples like this: when we add ¹/₁₆ ten times we ought to be able to get ¹⁰/₁₆.

1. The comment for Rational_Construct says that it returns:

    A Rational object X such that X.Top == Numerator and X.Bottom = Denominator.
   

but in fact this is not true, since the denominator may be negated. Instead it should say something like:

 A Rational object R such that R.Top/R.Bottom == Numerator/Denominator.

1. Your operator implementations don't call Rational_Construct: they just build a new Rational structure however they please. This means that the consistency checks in Rational_Construct (such as making sure that the denominator is positive) are skipped.

It would result in shorter and more reliable code if you always called Rational_Construct. For example:

 Rational Rational_Negate(const Rational R) {
 return Rational_Construct(-R.Top, R.Bottom);
 }
 Rational Rational_Add(const Rational Left, const Rational Right) {
 return Rational_Construct(Left.Top * Right.Bottom + Right.Top * Left.Bottom,
 Left.Bottom * Right.Bottom);
 }

1. Even small computations with your rationals result in integer overflow. For example, consider the following program:

    int main(int argc, char **argv) {
    Rational sixteenth = Rational_Construct(1, 16);
    Rational sum = Rational_Construct(0, 1);
    int i;
    for (i = 0; i < 10; ++i) {
    sum = Rational_Add(sum, sixteenth);
    printf("%d/%d\n", sum.Top, sum.Bottom);
    }
    return 0;
    }
   

Now, because you're using C's fixed-size integers, there are bound to be computations whose results are too large to be represented. But it would be better to raise an error when the result is too large, rather than causing undefined behaviour. And you should at the very least make some effort to ensure that integer overflow doesn't happen in small examples like this: when we add ¹/₁₆ ten times we ought to be able to get ¹⁰/₁₆.

An easy way to avoid integer overflow in small examples is to always reduce rationals to their lowest terms (thus turning ²/₄ as ¹/₂). For example, you could do this:

 #include <assert.h> /* for the assert() prototype */
 #include <limits.h> /* for INT_MIN */
 #include <stdlib.h> /* for the abs() prototype */
 Rational Rational_Construct(int Numerator, int Denominator) {
 /* Ensure that Denominator is positive */
 assert(Denominator != 0);
 if (Denominator < 0) {
 assert(Numerator != INT_MIN);
 assert(Denominator != INT_MIN);
 Numerator = -Numerator;
 Denominator = -Denominator;
 }
 /* Find the greatest common divisor of Numerator and Denominator. */
 int a = abs(Numerator), b = Denominator;
 while (b) {
 int c = a % b;
 a = b;
 b = c;
 }
 /* Reduce the fraction to lowest terms. */
 Rational newRational;
 newRational.Top = Numerator / a;
 newRational.Bottom = Denominator / a;
 return newRational;
 }

Now the program I gave above prints:

 1/16
 1/8
 3/16
 1/4
 5/16
 3/8
 7/16
 1/2
 9/16
 5/8

(Of course this doesn't solve the integer overflow problem in the general case, but at least it allows small programs like this to complete successfully.)