Adding and subtracting vulgar fractions

Question

I'm trying to add two vulgar fractions together by finding the lowest common denominator and then adding. However, my code isn't behaving as expected, and is outputting two very high negative numbers. When I change the second fraction to 3/15 it outputs 0/0.

Here is my main program code:

class Program
{
    static void Main(string[] args)
    {
        Fraction n = new Fraction(2, 4);
        Fraction z = new Fraction(3, 12);

        Fraction sum = n.Add(z, n);
        int num = sum.Numerator;
        int den = sum.Denominator;

        Console.WriteLine("{0}/{1}", num, den);
        Console.ReadKey(true);
    }
}

Here is my Fraction class code:

internal class Fraction
{
    public Fraction(int numerator, int denominator)
    {
        Numerator = numerator;
        Denominator = denominator;
    }

    public int Numerator { get; private set; }

    public int Denominator { get; private set; }

    public Fraction Add(Fraction fraction2, Fraction fraction8)
    {
        int lcd = GetLCD(fraction8, fraction2);

        int x = lcd/fraction8.Denominator;
        int n = lcd/fraction2.Denominator;
        int f2num = fraction2.Numerator*n;
        int f8num = fraction8.Numerator*x;

        int t = fraction2.Numerator;

        Fraction Fraction3 = new Fraction(f2num+f8num,lcd);

        return Fraction3;
    }



    public int GetLCD(Fraction b, Fraction c)
    {
        int i = b.Denominator;
        int j = c.Denominator;

        while (true)
        {

            if (i == j)
            {
                return i;
            }
            j = j + j;
            i = i + i;
        }
    }
}

Answer 1

Personally, I think your first mistake is trying to calculate the Lowest Common Denominator instead of just finding the simplest common denominator. Finding the LCD is a great stratgety for humans working this out on paper because of pattern recognition: we can recognize LCDs quickly; but calculating the LCD, and then converting the fractions to it is significantly more steps for a computer that must perform every step every time (and is not able to recognize patterns). Plus, once you add the two fractions after transforming them to their LCD it isn't even guaranteed to be a reduced result. I'm assuming that the reduced result is required, as is usually expected with fractional arithmetic. And because it seems useful, I put the reduction directly into the constructor code:

internal class Fraction
{
public Fraction(int numerator, int denominator, bool reduce = false)
{
    if (!reduce)
    {
        Numerator = numerator;
        Denominator = denominator;
    }
    else
    {
        var GCD = GreatestCommonDivisor(numerator, denominator);
        Numerator = numerator / GCD;
        Denominator = denominator / GCD;
    }
}

public int Numerator { get; private set; }

public int Denominator { get; private set; }    

public static Fraction Add(Fraction first, Fraction second)
{
    return Combine(first, second, false);
}

public static Fraction Subtract(Fraction first, Fraction second)
{
    return Combine(first, second, true);
}

private static Fraction Combine(Fraction first, Fraction second, bool isSubtract)
{
    var newDenominator = first.Denominator * second.Denominator;
    var newFirst = first.Numerator * second.Denominator;
    var newSecond = first.Denominator * second.Denominator;

    if (isSubtract)
    {
        newSecond = newSecond * -1;
    }

    return new Fraction(newFirst + newSecond, newDenominator, true);
}

private static int GreatestCommonDivisor(int a, int b) 
{
    return b == 0 ? a : GreatestCommonDivisor(b, a % b);
}

}

Edit: stole Greatest Common Divisor code from this answer

Answer 2

It didn't make sense to have GetLCD, Add & Subtract methods in the class. So, I moved it out of the class and made them static methods.

Your GetLCD function doesn't get LCD correctly. This will give you the required result.(I didn't bother to make the Subtract method working, you can follow the below code & make it work yourself)

PS: I didn't change all of your variable names & I would recommend you to make them as meaningful as possible. n,z,x,y,b,c are not good variable names

    static void Main(string[] args)
    {
        Fraction n = new Fraction(2, 4);
        Fraction z = new Fraction(3, 12);

        Fraction sum = Add(z, n);
        int x = sum.Numerator;
        int y = sum.Denominator;

        Console.WriteLine("{0}/{1}", x, y);
        Console.ReadKey(true);
    }

    public static Fraction Add(Fraction fraction2, Fraction fraction8)
    {

        int lcd = GetLCD(fraction8, fraction2);
        int multiplier = 0;
        if (fraction2.Denominator < lcd)
        {
            multiplier = lcd / fraction2.Denominator;
            fraction2.Numerator = multiplier * (fraction2.Numerator);
            fraction2.Denominator = multiplier * (fraction2.Denominator);
        }
        else
        {
            multiplier = lcd / fraction8.Denominator;
            fraction8.Numerator = multiplier * (fraction8.Numerator);
            fraction8.Denominator = multiplier * (fraction8.Denominator);
        }

        Fraction Fraction3 = new Fraction(fraction2.Numerator + fraction8.Numerator, lcd);

        return Fraction3;
    }

    public static int GetLCD(Fraction b, Fraction c)
    {
        int i = b.Denominator;
        int j = c.Denominator;

        int greater = 0;
        int lesser = 0;

        if (i > j)
        {
            greater = i; lesser = j;
        }
        else if (i < j)
        {
            greater = j; lesser = i;
        }
        else
        {
            return i;
        }
        for (int iterator = 1; iterator <= lesser; iterator++)
        {
            if ((greater * iterator) % lesser == 0)
            {
                return iterator * greater;
            }
        }
        return 0;
    }

    internal class Fraction
    {
        public Fraction(int numerator, int denominator)
        {
            Numerator = numerator;
            Denominator = denominator;
        }
        public int Numerator { get; set; }
        public int Denominator { get; set; }
    }