Finding the lowest terms for fractions in C#

Question

I have been working on this Rational Operations calculator where you can input two rational numbers and can add, subtract, multiply and divide and the final output is the reduced answer.

For example:

\$\dfrac{3}{4} \times \dfrac{8}{15}\$ and the output would be \$\dfrac{2}{5}\$.

I have all but the reducing done i have some code in there that I think might work but I am brain dead from working on this and can't think were it would go or if it would even work. I am using classes so will post both sets of code.

Class part:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Assignment_5
{
    class Assignment5
    {
        #region Fields
        private int fiNum;
        private int fiDenom;
        private bool fbValid;
        #endregion

        #region Constructor
        public Assignment5(string strRat)//constructor, make it a public and name shoud have the same name of the class
        {
            Parse(strRat);
        }
        #endregion
        #region Methods
        public override string ToString()
        {
            string strRat;
            if (!fbValid)
                strRat = "NaN";     //if not a valid number
            else
            {
                strRat = fiNum.ToString();
                if (fiDenom > 1)            //if denom is 1 only output numbertor
                    strRat += "/" + fiDenom.ToString();
            }
            return strRat;
        }
        public void Parse(string strRat)
        {
            bool bNeg = false;
            bool bSlash = false;
            bool bLegal = true;
            string strDenom = "0";
            string strNum = "0";

            for (int i = 0; i < strRat.Length; i++)  // **** RETURN LENGTH OF THE STRING ****
            {
                if (!Char.IsDigit(strRat[i]))                   //boolean checks if charactor is a digit
                {
                    if (i == 0 && strRat[i] == '-')     //checks if it is negative
                        bNeg = true;
                    else if (i > 0 && strRat[i] == '/' && !bSlash)  //needing another boolean to check
                        bSlash = true;
                    else
                        bLegal = false;
                }
                else
                {
                    if (bSlash)
                        strDenom += strRat[i];
                    else
                        strNum += strRat[i];
                }
                if (bLegal)
                {
                    fiNum = int.Parse(strNum);
                    if (bNeg)
                        fiNum *= -1;
                    fiDenom = int.Parse(strDenom);
                    if (!bSlash)
                        fiDenom = 1;
                    if (fiDenom == 0)
                        fbValid = false;
                    else
                        fbValid = true;
                }
                else
                    fbValid = false;
            }
        #endregion
        }

        //adding the two fractions together 
        public static Assignment5 operator +(Assignment5 rat1, Assignment5 rat2)
        {
            if (rat1.fbValid && rat2.fbValid)
            {
                rat1.fiNum = rat1.fiNum * rat2.fiDenom + rat1.fiDenom * rat2.fiNum;
                rat1.fiDenom = rat1.fiDenom * rat2.fiDenom;
            }
            else rat1.fbValid = false;
            return rat1;
        }

        //subtracting the two fractions 
        public static Assignment5 operator -(Assignment5 rat1, Assignment5 rat2)
        {
            if (rat1.fbValid && rat2.fbValid)
            {
                rat1.fiNum = rat1.fiNum * rat2.fiDenom - rat1.fiDenom * rat2.fiNum;
                rat1.fiDenom = rat1.fiDenom * rat2.fiDenom;
            }
            else rat1.fbValid = false;
            return rat1;
        }

        //multiplying the fractions 
        public static Assignment5 operator *(Assignment5 rat1, Assignment5 rat2)
        {
            if (rat1.fbValid && rat2.fbValid)
            {
                rat1.fiNum = rat1.fiNum * rat2.fiNum;
                rat1.fiDenom = rat1.fiDenom * rat2.fiDenom;
            }
            else rat1.fbValid = false;
            return rat1;
        }

        //dividing the fractions 
        public static Assignment5 operator /(Assignment5 rat1, Assignment5 rat2)
        {
            if (rat1.fbValid && rat2.fbValid)
            {
                rat1.fiNum = rat1.fiNum * rat2.fiDenom;
                rat1.fiDenom = rat1.fiDenom * rat2.fiNum;
            }
            else rat1.fbValid = false;
            return rat1;
        }
        //DONT KNOW IF THIS PART WORK AND THIS IS WERE I ENDED HELP!!!!
        public static int gcd(int rat1, int rat2)
        {
            if (rat2 == 0) return rat1;
            else
                return gcd(rat2, rat1 % rat2);
        }

    }
} 

Form part:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace Assignment_5
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }
        #region
        private void Form1_Load(object sender, EventArgs e)
        {

        }
        #endregion  //blank form1 Load

        //Add button code
        private void btnAdd_Click(object sender, EventArgs e)
        {
            Assignment5 ratNum1, ratNum2, ratSum;      //declares new rational object
            ratNum1 = new Assignment5(txtRat1.Text);  //calls constructor           
            ratNum2 = new Assignment5(txtRat2.Text);
            ratSum = new Assignment5("0/1");

            ratSum = ratNum1 + ratNum2;
            txtSum.Text = ratSum.ToString();
        }

        //Subtract button code
        private void btnSub_Click(object sender, EventArgs e)
        {
            Assignment5 ratNum1, ratNum2, ratSum;      //declares new rational object
            ratNum1 = new Assignment5(txtRat1.Text);  //calls constructor           
            ratNum2 = new Assignment5(txtRat2.Text);
            ratSum = new Assignment5("0/1");

            ratSum = ratNum1 - ratNum2;
            txtSum.Text = ratSum.ToString();
        }

        //Multiply button code
        private void btnMultiply_Click(object sender, EventArgs e)
        {
            Assignment5 ratNum1, ratNum2, ratSum;      //declares new rational object
            ratNum1 = new Assignment5(txtRat1.Text);  //calls constructor           
            ratNum2 = new Assignment5(txtRat2.Text);
            ratSum = new Assignment5("0/1");

            ratSum = ratNum1 * ratNum2;           
            txtSum.Text = ratSum.ToString();
        }

        //Divide button code
        private void btnDivd_Click(object sender, EventArgs e)
        {
            Assignment5 ratNum1, ratNum2, ratSum;      //declares new rational object
            ratNum1 = new Assignment5(txtRat1.Text);  //calls constructor           
            ratNum2 = new Assignment5(txtRat2.Text);
            ratSum = new Assignment5("0/1");

            ratSum = ratNum1 / ratNum2;
            txtSum.Text = ratSum.ToString();
        }
    }
}

Answer 1

Your event handlers can be greatly simplified since they all have duplicate code. You can use the Tag property to tell what operation to perform. Then simply assign the same method to handle each event. something like this:

public Form1()
{
    InitializeComponent();
    btnAdd.Tag = '+';
    btnSub.Tag = '-';
    btnMultiply.Tag = '*';
    btnDivd.Tag = '/';
}

private void btn_Click(object sender, EventArgs e)
{
    var clickedButton = (Button)sender;
    Assignment5 ratNum1, ratNum2, ratSum;      //declares new rational object
    ratNum1 = new Assignment5(txtRat1.Text);  //calls constructor           
    ratNum2 = new Assignment5(txtRat2.Text);
    ratSum = new Assignment5("0/1");
    switch ((char)clickedButton.Tag)
    {
        case '+':
            {
                ratSum = ratNum1 + ratNum2;
                break;
            }
        case '-':
            {
                ratSum = ratNum1 - ratNum2;
                break;
            }
        case '*':
            {
                ratSum = ratNum1 * ratNum2;
                break;
            }
        case '/':
            {
                ratSum = ratNum1 / ratNum2;
                break;
            }
    }
    txtSum.Text = ratSum.ToString();
}

Answer 2

Try to give your variables and types better names and maintain consistent naming.

Better parse with regular expressions.

fbValid is not good. It's better to prevent instances in invalid state to exist.

When overriding operator, don't modify input values. Create a new instance (result of an operation) and return it.

public class RationalNumber
{
    private static Regex _pattern = new Regex(@"(-?)(\d+)(/(\d+))?");

    public int Numerator { get; private set; }
    public int Denominator { get; private set; }

    public static bool TryParse(string input, out RationalNumber result)
    {
        var match = _pattern.Match(input);

        try
        {
            var denominator = match.Groups[3].Value == string.Empty ? 1 : int.Parse(match.Groups[4].Value);

            if (denominator == 0)
            {
                result = null;
                return false;
            }

            var numerator = int.Parse(match.Groups[2].Value) * (match.Groups[1].Value == string.Empty ? 1 : -1);

            result = new RationalNumber
            {
                Numerator = numerator,
                Denominator = denominator
            }.Simplify();

            return true;
        }
        catch
        {
            result = null;            
        }
        return false;
    }

    public override string ToString()
    {
        return Denominator == 1 ? Numerator.ToString() 
                                : string.Format("{0}/{1}", Numerator, Denominator);
    }

    public static RationalNumber operator +(RationalNumber rat1, RationalNumber rat2)
    {
        ThrowIfArgumentsNull(rat1, rat2);

        return new RationalNumber
        {
            Numerator = rat1.Numerator * rat2.Denominator + rat1.Denominator * rat2.Numerator,
            Denominator = rat1.Denominator * rat2.Denominator
        }.Simplify();
    }

    public static RationalNumber operator -(RationalNumber rat1, RationalNumber rat2)
    {
        ThrowIfArgumentsNull(rat1, rat2);

        return new RationalNumber
        {
            Numerator = rat1.Numerator * rat2.Denominator - rat1.Denominator * rat2.Numerator,
            Denominator = rat1.Denominator * rat2.Denominator
        }.Simplify();
    }

    public static RationalNumber operator *(RationalNumber rat1, RationalNumber rat2)
    {
        ThrowIfArgumentsNull(rat1, rat2);

        return new RationalNumber
        {
            Numerator = rat1.Numerator * rat2.Numerator,
            Denominator = rat1.Denominator * rat2.Denominator
        }.Simplify();
    }

    public static RationalNumber operator /(RationalNumber rat1, RationalNumber rat2)
    {
        ThrowIfArgumentsNull(rat1, rat2);

        return new RationalNumber
        {
            Numerator = rat1.Numerator * rat2.Denominator,
            Denominator = rat1.Denominator * rat2.Numerator
        }.Simplify();
    }

    private static void ThrowIfArgumentsNull(RationalNumber rat1, RationalNumber rat2)
    {
        if (rat1 == null || rat2 == null)
        {
            throw new ArgumentException("Argument must not be null");
        }
    }

    private RationalNumber Simplify()
    {
        var greatestCommonDivisor = GreatestCommonDivisor(Numerator, Denominator);
        Numerator /= greatestCommonDivisor;
        Denominator /= greatestCommonDivisor;
        return this;
    }

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

Answer 3

Your original code contains an apparently correct implementation of Euclid's Algorithm, as do some of the other answers, but it seems you weren't sure if the algorithm was correct, so I'll spend some time to explain it a little. (aush gives a very terse C# implementation of the algorithm in his answer as the private method GreatestCommmonDivisor, which is great for writing the program, but not so good for understanding the algorithm.) I don't know C#, but I took number theory at university, the first half of which was more like "Applied Euclid's Algorithm", so I'll summarize the idea behind it a little.

The idea is that, when you reduce a fraction into lowest terms, you're dividing out the greatest common divisor of the numerator and denominator. In your example, \$ \frac{3}{4} \times \frac{8}{15} = \frac{24}{60} = \frac{2\times12}{5\times12}\$. Euclid's Algorithm can find the GCD of the numerator and denominator.

Think of writing out the numerator and denominator as prime factorizations: \$\frac{24}{60} = \frac{2\times2\times2\times3}{2\times2\times3\times5}\$. Euclid's Algorithm finds the number whose prime factorization contains all the prime factors shared between the numerator and denominator, in this case \$12 = 2\times2\times3\$. When you divide out the GCD, you're dividing out all the shared prime factors: $$\frac{\frac{24}{12}}{\frac{60}{12}} = \frac{\frac{2\times2\times2\times3}{2\times2\times3}}{\frac{2\times2\times3\times5}{2\times2\times3}} = \frac{2}{5}$$.

There are no more shared prime factors, so the fraction can't be reduced any more. In this case, the numerator and denominator are both prime and are unequal, so it's obvious there are no more shared factors. Other times, you might have two composite numbers that share no prime factors, e.g. \$\frac{8}{15} = \frac{2\times2\times2}{3\times5}\$.

The idea behind Euclid's algorithm is this: if we have some numbers \$a\$ and \$b\$, both integers, and \$a > b\$, we can divide \$a\$ by \$b\$ and get two numbers, a quotient \$q\$ and a remainder \$r\$ (remember back in grade school when \$23 / 11\$ was something like \$2 \ \mathrm{remainder} \ 1\$?). In other words, \$a = bq + r\$. There's a theorem that if some number \$d\$ is the greatest common divisor of \$a\$ and \$b\$, then \$d\$ is also the greatest common divisor of \$b\$ and \$r\$. So we calculate \$r\$, and then we feed it back into the algorithm as the new \$b\$, with the old \$b\$ as the new \$a\$. Eventually, we'll get a remainder of \$0\$. The value of \$a\$ on this final iteration is the GCD of the original \$a\$ and \$b\$ that we started with. (All of this can be proven, but I won't do that here. Here's a page that both describes the algorithm in more detail and gives a proof for it.)

I hope this answer and the pages I linked can help you understand that the GCD method does work, and why. Here's a link to another page that also explains it in terms of code. The code is in C, but shouldn't be too hard to understand if you know C#.

If you're interested, you can also use Euclid's algorithm to compute the least common multiple of two numbers, which could be useful in the addition and subtraction methods. From Wikipedia:

$$\mathrm{lcm}(a, b) = \frac{\vert ab \vert}{\gcd(ab)}$$

So you could also write your addition method something like this (this is the first C# I've ever written, so there might be some errors):

//adding the two fractions together 
public static Ratio operator +(Ratio rat1, Ratio rat2)
{
  // With appropriate error checking
  int lcm = Math.abs(rat1.fiDenom * rat2.fiDenom) / gcd(rat1.fiDenom, rat2.fiDenom);
  int mult1 = rat1.fiDenom / lcm;
  int mult2 = rat2.fiDenom / lcm;
  return new Ratio(rat1.fiNum*mult1 + rat2.fiNum*mult2, lcm);
}

The advantage is that your result is already completely reduced to lowest terms.

Answer 4

Naming

Names of classes, methods and variables should be as descriptive and meaningful as possible. Assume Sam the Maintainer has to fix a bug/unexpected behaviour in your code and sees classes named Assignment5 and variables named fiNum, strRat, ... Sam the Maintainer would think wtf is going on in this code.

Grasping the context of the code at first glance should be the top priority while writing code.

Speaking about Assignment5, if you would have named the class Fraction or Rational this and some more comments would have been superfluous

//adding the two fractions together 
public static Assignment5 operator +(Assignment5 rat1, Assignment5 rat2)

---

Comments

Comments should describe why something is done in the way it is done. Let the code speak for itself about what is done by using meaningful names.

Comments like

//blank form1 Load
//calls constructor
//Subtract button code  

doesn't add any value to the code. This is just noise which should be removed.

---

Style

If you have choosen a style to code something e.g if..else you should stick to that style. Right now you are mixing different styles for the same thing.

if (fiDenom == 0)
    fbValid = false;
else
    fbValid = true; 

sometimes you place the statement after the if and else on a new line,

if (rat2 == 0) return rat1;
else
    return gcd(rat2, rat1 % rat2);  

sometimes you place it on the same line as the if and on a new line for the else,

if (rat1.fbValid && rat2.fbValid)
{
    rat1.fiNum = rat1.fiNum * rat2.fiDenom + rat1.fiDenom * rat2.fiNum;
    rat1.fiDenom = rat1.fiDenom * rat2.fiDenom;
}
else rat1.fbValid = false;  

and sometimes you place it on the same line as the else.

Much better IMHO would be to always use braces {} to make your code less error prone. IMHO it is also easier to read, because it is always the same style.

---

Declaring multiple variables on the same line reduces readability. You should always try to improve the readability of the code. So instead of

Assignment5 ratNum1, ratNum2, ratSum;  
ratNum1 = new Assignment5(txtRat1.Text);  //calls constructor         
ratNum2 = new Assignment5(txtRat2.Text); 
ratSum = new Assignment5("0/1");

you should use

Assignment5 ratNum1 = new Assignment5(txtRat1.Text);
Assignment5 ratNum2 = new Assignment5(txtRat2.Text);
Assignment5 ratSum = new Assignment5("0/1");  

---

Read about the using of region's here: are-regions-an-antipattern-or-code-smell.