A fraction of the code

Following up on this post, and including some major changes suggested, here's the revised code.

Changes include:

• No longer keeping an IFormatProvider at instance level.

• Removed IFormatProvider constructor parameters.

• Introduced ToString(IFormatProvider) overload.

• Changed decimal to float, to leverage float.NaN for 0-denominator fractions.

• Implemented more operators.

• Reimplemented Equals and CompareTo per recommendations.

• Fixed a bug in ToString(string, IFormatProvider).

• Added XML comments on all public members.

As the code file grew, it became apparent that I was going to need a way of grouping code sections. I did not want to use #region because... it's a question of principles. It's irrational, I just don't want to use #region.

So I regrouped all static members into a partial struct (note: some code lines and XML comments were reformatted to avoid horizontal scrolling):

```
///
/// A fractional representation of a rational number.
///
public partial struct Fraction
{
///
/// An empty Fraction (0/0).
///
public static readonly Fraction Empty = new Fraction();

///
/// A Fraction representation of the integer value 0.
///
public static readonly Fraction Zero = new Fraction(default(int));

///
/// A Fraction representation of the integer value 1.
///
public static readonly Fraction One = new Fraction(1);

///
/// Represents the smallest possible value for a .
///
public static readonly Fraction MinValue = new Fraction(1, int.MinValue);

///
/// Represents the largest possible value for a .
///
public static readonly Fraction MaxValue = new Fraction(int.MaxValue, 1);

///
/// Returns a simplified/reduced representation of a fraction.
///
/// The fraction to simplify.
///
/// Returns a new ,
/// a simplified representation of this instance (if simplification is possible).
///
public sta

It looks good! Here are some issues I found.

Readability

I would recommend 0 over default(int) for readability.

Naming

Greatest common divisor is a more standard term for greatest common denominator.

MinValue

Fraction.MinValue should be new Fraction(int.MinValue, 1), not new Fraction(1, int.MinValue).

Immutability

Since your data structure is immutable, this code

public static Fraction Simplify(Fraction fraction)
{
    if (fraction.IsUndefined)
    {
        return new Fraction(fraction);
    }
    ...

can be

public static Fraction Simplify(Fraction fraction)
{
    if (fraction.IsUndefined)
    {
        return fraction;
    }
    ...

++ and --

From MSDN:


The increment operator (++) increments its operand by 1.

However, the way the increment and decrement operators are written, they increment (decrement) by 1 / denominator. This leads to confusing behaviour.

For example, how can we make this assertion fail?

if (x == y)
{
    x++;
    y++;
    Debug.Assert(x == y);
}

Just set

var x = new Fraction(2, 4);
var y = x.Simplify();

After incrementing, we have x = 3/4 and y = 2/2.

Or this contrived example will fail for x = new Fraction(1, 2):

var y = x;
// Waste time by doing nothing to x.
x++;
x *= 1;
x--;
// Make sure we didn't change x.
Debug.Assert(x == y);

I would stick to the guidelines and increment (decrement) by 1.

TryParse

You probably want to use the character class [0-9] instead of \d in your regular expression. This is because \d matches Unicode decimal digits. So this code will throw a FormatException:

Fraction f;
if (Fraction.TryParse("١/٢", out f))
{
    Console.WriteLine(f);
}

Floats

Comparing to floats is... tricky.

float oneThird = 1.0f / 3;
Console.WriteLine(new Fraction(1, 3) == oneThird);
> False

(Oh, and good call on not using #region.)

StackExchange Code Review Q#56968, answer score: 11