11
\$\begingroup\$

I have written this small little program, to test how my Fraction type behaves when used with and without a custom FractionFormatter - I've implemented two custom formatters:

  • FractionFormatter, which simply puts the numerator at the left of a slash, and the denominator at the right ("{0}/{1}", fraction.Numerator, fraction.Denominator),
  • MathJaxFractionFormatter, pushes the customization a bit further by formatting the fraction as MathJax, so 2/5 could be rendered as \$\frac{2}{5}\$ which produces \$\frac{2}{5}\$ in CR posts.
static void Main(string[] args)
{
    var fraction1 = new Fraction(2);
    var fraction2 = new Fraction(2, 4);

    Console.WriteLine("Fraction1: {0} (decimal: {1})", fraction1, fraction1.ToDecimal());
    Console.WriteLine("Fraction2: {0} (decimal: {1})", fraction2, fraction2.ToDecimal());

    Console.WriteLine("{0} + {1} = {2} (decimal: {3})", fraction1, fraction2, fraction1 + fraction2, (fraction1 + fraction2).ToDecimal());
    Console.WriteLine("{0} - {1} = {2} (decimal: {3})", fraction1, fraction2, fraction1 - fraction2, (fraction1 - fraction2).ToDecimal());
    Console.WriteLine("{0} * {1} = {2} (decimal: {3})", fraction1, fraction2, fraction1 * fraction2, (fraction1 * fraction2).ToDecimal());
    Console.WriteLine("{0} / {1} = {2} (decimal: {3})", fraction1, fraction2, fraction1 / fraction2, (fraction1 / fraction2).ToDecimal());

    var jaxFormatter = new MathJaxFractionFormatter();
    var fraction3 = new Fraction(2, jaxFormatter);
    var fraction4 = new Fraction(2, 4, jaxFormatter);

    Console.WriteLine("Fraction3: {0} ({1})", fraction3, fraction3.ToString(null, new FractionFormatter()));
    Console.WriteLine("Fraction4: {0} ({1})", fraction4, fraction4.ToString(null, new FractionFormatter()));

    var crJaxFormatter = new MathJaxFractionFormatter("\\$", MathJaxFractionFormatter.MathJaxFractionSize.Large);
    var fraction5 = new Fraction(2, crJaxFormatter);
    var fraction6 = new Fraction(2, 4, crJaxFormatter);

    Console.WriteLine("Fraction5: {0}", fraction5);
    Console.WriteLine("Fraction6: {0}", fraction6);

    Console.ReadLine();
}

The sandbox program above outputs this:

Fraction1: 2/1 (decimal: 2)
Fraction2: 2/4 (decimal: 0.5)
2/1 + 2/4 = 5/2 (decimal: 2.5)
2/1 - 2/4 = 3/2 (decimal: 1.5)
2/1 * 2/4 = 1/1 (decimal: 1)
2/1 / 2/4 = 4/1 (decimal: 4)
Fraction3: $\frac{2}{1}$ (2/1)
Fraction4: $\frac{2}{4}$ (2/4)
Fraction5: \$\dfrac{2}{1}\$
Fraction6: \$\dfrac{2}{4}\$

The Fraction type was largely inspired (ok, stolen) from this Objective-C question - my implementation is an immutable struct though:

[Serializable]
public struct Fraction : IFormattable,
                         IComparable, 
                         IComparable<Fraction>,
                         IEquatable<Fraction>
{
    private readonly IFormatProvider _formatProvider;
    private readonly int _numerator;
    private readonly int _denominator;

    public Fraction(int numerator) 
        : this(numerator, 1) 
    {
    }

    public Fraction(int numerator, int denominator)
        : this(numerator, denominator, null)
    {
    }

    public Fraction(int numerator, IFormatProvider formatProvider)
        : this(numerator, 1, formatProvider)
    {
    }

    public Fraction(int numerator, int denominator, IFormatProvider formatProvider)
    {
        _numerator = numerator;
        _denominator = denominator;

        _formatProvider = formatProvider ?? new FractionFormatter();
    }

    public int Numerator { get { return _numerator; } }
    public int Denominator { get { return _denominator; } }

    public Fraction Simplify()
    {
        var greatestCommonDenominator = GetGreatestCommonDenominator(_numerator, _denominator);

        var numerator = _numerator / greatestCommonDenominator;
        var denominator = _denominator / greatestCommonDenominator;

        return new Fraction(numerator, denominator);
    }

    public decimal ToDecimal()
    {
        return (decimal)_numerator / (decimal)_denominator;
    }

    private int GetGreatestCommonDenominator(int numerator, int denominator)
    {
        return denominator == 0 ? numerator 
                           : GetGreatestCommonDenominator(denominator, numerator % denominator);
    }

    public static explicit operator decimal (Fraction fraction)
    {
        return fraction.ToDecimal();
    }

    public static Fraction operator +(Fraction fraction, int integer)
    {
        return fraction + new Fraction(integer);
    }

    public static Fraction operator +(Fraction fraction1, Fraction fraction2)
    {
        int numerator = (fraction1.Numerator * fraction2.Denominator) + (fraction1.Denominator * fraction2.Numerator);
        int denominator = (fraction1.Denominator * fraction2.Denominator);

        var result = new Fraction(numerator, denominator).Simplify();
        return result;
    }

    public static Fraction operator -(Fraction fraction, int integer)
    {
        return fraction - new Fraction(integer);
    }

    public static Fraction operator -(Fraction fraction1, Fraction fraction2)
    {
        var subtrator = new Fraction(fraction2.Numerator*-1, fraction2.Denominator);
        return fraction1 + subtrator;
    }

    public static Fraction operator /(Fraction fraction, int integer)
    {
        return fraction / new Fraction(integer);
    }

    public static Fraction operator /(Fraction fraction1, Fraction fraction2)
    {
        var divisor = new Fraction(fraction2.Denominator, fraction2.Numerator);
        return fraction1 * divisor;
    }

    public static Fraction operator *(Fraction fraction, int integer)
    {
        return fraction * new Fraction(integer);
    }

    public static Fraction operator *(Fraction fraction1, Fraction fraction2)
    {
        var numerator = fraction1.Numerator * fraction2.Numerator;
        var denominator = fraction1.Denominator * fraction2.Denominator;

        var result = new Fraction(numerator, denominator).Simplify();
        return result;
    }

    public override bool Equals(object obj)
    {
        return ToDecimal().Equals((decimal)obj);
    } 

    public override int GetHashCode()
    {
        return ToDecimal().GetHashCode();
    }

    public override string ToString()
    {
        return ToString(null, _formatProvider);
    }


    public string ToString(string format, IFormatProvider formatProvider)
    {
        if (formatProvider is ICustomFormatter)
        {
            return ((ICustomFormatter)formatProvider).Format(format, this, formatProvider);
        }
        else
        {
            return ToString();
        }
    }

    public int CompareTo(object obj)
    {
        if (obj is int)
        {
            return CompareTo(new Fraction((int)obj, 1));
        }
        else if (obj is string)
        {
            int intValue;
            if (int.TryParse(obj as string, out intValue))
            {
                return CompareTo(new Fraction(intValue));
            }
        }

        // will throw an InvalidCastException when obj cannot be cast to a Fraction:
        return CompareTo((Fraction)obj);
    }

    public int CompareTo(Fraction other)
    {
        return ToDecimal().CompareTo(other.ToDecimal());
    }

    public bool Equals(Fraction other)
    {
        return ToDecimal().Equals(other.ToDecimal());
    }
}

The FractionFormatter:

public class FractionFormatter : IFormatProvider, ICustomFormatter
{
    private static readonly CultureInfo _culture = typeof(FractionFormatter).Assembly.GetName().CultureInfo;

    public object GetFormat(Type formatType)
    {
        return (formatType == typeof(ICustomFormatter)) ? this : null;
    }

    public string Format(string format, object arg, IFormatProvider formatProvider)
    {
        var fraction = (Fraction)arg;
        if (string.IsNullOrEmpty(format))
        {
            return string.Format(_culture, "{0}/{1}", fraction.Numerator, fraction.Denominator);
        }
        else
        {
            return fraction.ToString(format, _culture);
        }
    }
}

And the MathJaxFractionFormatter:

public class MathJaxFractionFormatter : IFormatProvider, ICustomFormatter
{
    public enum MathJaxFractionSize
    {
        Normal,
        Large
    }

    private static readonly CultureInfo _culture = typeof(FractionFormatter).Assembly.GetName().CultureInfo;

    private readonly string _delimiter;
    private readonly MathJaxFractionSize _size;

    public MathJaxFractionFormatter()
        : this("$", MathJaxFractionSize.Normal) { }

    public MathJaxFractionFormatter(string delimiter, MathJaxFractionSize size)
    {
        _delimiter = delimiter;
        _size = size;
    }

    public object GetFormat(Type formatType)
    {
        return (formatType == typeof(ICustomFormatter)) ? this : null;
    }

    public string Format(string format, object arg, IFormatProvider formatProvider)
    {
        var fraction = (Fraction)arg;
        if (string.IsNullOrEmpty(format))
        {
            var keyword = _size == MathJaxFractionSize.Normal ? "\\frac" : "\\dfrac";
            return string.Format(_culture, "{2}{3}{{{0}}}{{{1}}}{2}", fraction.Numerator, fraction.Denominator, _delimiter, keyword);
        }
        else
        {
            return fraction.ToString(format, _culture);
        }
    }
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ How do you handle 0 denominators? I'm not familiar with C# and didn't see anywhere that this was handled. \$\endgroup\$
    – nhgrif
    Jul 13, 2014 at 13:26
  • \$\begingroup\$ I... somehow let that slip... ..it'll throw a DivideByZeroException whenever ToDecimal gets called, ...which means GetHashCode can't run for such an instance! \$\endgroup\$ Jul 13, 2014 at 14:08

1 Answer 1

7
\$\begingroup\$

Immutable structs

Yay! ✓✓✓

Format provider

It seems odd to see a format provider in a constructor, and stranger still to store a format provider for every fraction instance. Think of the memory overhead -- imagine a use case where I perform calculations on millions of fractions, but only want to print the end result.

Whoops

This will not terminate:

Console.WriteLine(new Fraction(1, 1, CultureInfo.InvariantCulture));

Edge cases

As @nhgrif hinted at, you should throw an ArgumentOutOfRange exception in the constructor for a zero denominator.

As @nhgrif mentioned, you need to be careful handling zero denominators. For example,

Console.WriteLine(new Fraction(1) / new Fraction(1, 0));
0/1

Recursion

GetGreatestCommonDenominator can be written iteratively, instead of recursively.

Culture

Why are you getting the CultureInfo from the assembly? I'm not sure, but I think it would be better to use CultureInfo.CurrentCulture.

API

Add a ToString(IFormatProvider) method so you don't have to call it with a first argument of null.

You might also consider providing MaxValue, MinValue, Zero, and One fields.

Comparing fractions

Though I can't say it's wrong, it does feel weird to use floating-point values to compare fractions:

public int CompareTo(Fraction other)
{
    return ToDecimal().CompareTo(other.ToDecimal());
}

I would expect something like this:

public int CompareTo(Fraction other)
{
    // TODO: handle case where this and/or other have zero denominator.
    long lhs = _numerator * other._denominator;
    long rhs = other._numerator * _denominator;
    return lhs.CompareTo(rhs);
}

Then,

public bool Equals(Fraction other)
{
    return ToDecimal().Equals(other.ToDecimal());
}

could be

public bool Equals(Fraction other)
{
    return this.CompareTo(other) == 0;
}

Nit-picking

Finally, though it's a matter of preference, I would remove the redundant elses, e.g.

if (obj is int)
{
    return CompareTo(new Fraction((int)obj, 1));
}
else if (obj is string)
{
...
\$\endgroup\$
3
  • \$\begingroup\$ Good catch on CultureInfo.InvariantCulture - looks like this is where not writing unit tests is biting me! I don't agree with throwing an exception in the constructor for a zero-denominator. Reason is because it's a struct, not a class - one could always do var fraction = new Fraction(); and I'd expect ToString() to return "0/0" for that value. I have already modified this code quite a lot, thanks for your answer! I'll post a follow-up question soon-ish, stay tuned! \$\endgroup\$ Jul 14, 2014 at 0:36
  • \$\begingroup\$ Good point about var fraction = new Fraction();, and I don't know the best solution. Maybe ToString should return Undefined for a denominator of 0. \$\endgroup\$
    – mjolka
    Jul 14, 2014 at 0:57
  • 1
    \$\begingroup\$ In my new code I have a public bool IsUndefined { get { return _denominator == default(int); } } property, and public static readonly Fraction Empty = new Fraction(default(int));. I like your CompareTo implementations a lot, in my new code I've changed decimal for float and use float.NaN when IsUndefined is true, using "native C#" comparisons for handling the non-number without throwing. \$\endgroup\$ Jul 14, 2014 at 1:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.