Fraction calculator

[yoked88]

My first program implementing functions, reference parameters, abstraction. My main focus here was to keep all variables and calculations out of main.

#include <iostream>

using namespace std;

void menuFunction();
void addFraction(int, int, int, int, int&, int&);
void subtractFraction(int, int, int, int, int&, int&);
void multiplyFraction(int, int, int, int, int&, int&);
void divideFraction(int, int, int, int, int&, int&);

int main()
{
	menuFunction();
	return 0;
}

void menuFunction()
{
	int choice, num1, num2, denom1, denom2, numResult = 0, denomResult = 0;
	cout<<"This program lets you perform operations on fractions."<<endl;
	cout<<"Input data as (a/b) and (c/d), then select an operation to perform."<<endl<<endl;
	cout<<"Operation                Number"<<endl;
	cout<<"-------------------------------"<<endl;
	cout<<"Adding fractions:           1"<<endl;
	cout<<"Subtracting fractions:      2"<<endl;
	cout<<"Multiplying fractions:      3"<<endl;
	cout<<"Dividing fractions:         4"<<endl;
	cout<<"-------------------------------"<<endl;
	cout<<"\nEnter fractions:  "<<endl;
	cin>>num1>>denom1>>num2>>denom2;
	cout<<"Enter operation (number):   "<<endl;
	cin>>choice;
	cout<<endl;
	switch(choice)
	{
	case 1:
		addFraction(num1, denom1, num2, denom2, numResult, denomResult);
		break;
	case 2:
		subtractFraction(num1, denom1, num2, denom2, numResult, denomResult);
		break;
	case 3:
		multiplyFraction(num1, denom1, num2, denom2, numResult, denomResult);
		break;
	case 4:
		divideFraction(num1, denom1, num2, denom2, numResult, denomResult);
		break;
	default:
		cout<<"\nInvalid input.\n"<<endl;
	}
	cout<<"Result = "<<numResult<<"/"<<denomResult<<endl;
}

void addFraction(int top1, int bottom1, int top2, int bottom2, int& topResult, int& bottomResult)
{
	if(bottom1 != 0 && bottom2 != 0)
	{
	    int numerator1 = (bottom1 * bottom2) / bottom1;
	    int numerator2 = (bottom1 * bottom2) / bottom2;
	    bottomResult = bottom1 * bottom2;
	    int newNum1 = numerator1 * top1;
	    int newNum2 = numerator2 * top2;
	    topResult = newNum1 + newNum2;
	}
	else
		cout<<"\nInvalid input, denominator cannot be 0."<<endl;
}

void subtractFraction(int top1, int bottom1, int top2, int bottom2, int& topResult, int& bottomResult)
{
	if(bottom1 != 0 && bottom2 != 0)
	{
		int numerator1 = (bottom1 * bottom2) / bottom1;
		int numerator2 = (bottom1 * bottom2) / bottom2;
		bottomResult = bottom1 * bottom2;
		int newNum1 = numerator1 * top1;
		int newNum2 = numerator2 * top2;
		topResult = newNum1 - newNum2;
	}
	else
		cout<<"\nInvalid input, denominator cannot be 0."<<endl;
}

void multiplyFraction(int top1, int bottom1, int top2, int bottom2, int& topResult, int& bottomResult)
{
	if(bottom1 != 0 && bottom2 != 0)
	{
		bottomResult = bottom1 * bottom2;
		topResult = top1 * top2;
	}
	else
		cout<<"\nInvalid input, denominator cannot be 0."<<endl;
}

void divideFraction(int top1, int bottom1, int top2, int bottom2, int& topResult, int& bottomResult)
{
	if(bottom1 != 0 && bottom2 != 0)
	{
	    int newTop = bottom2;
	    int newBottom = top2;
	    topResult = top1 * newTop;
	    bottomResult = bottom1 * newBottom;
	}
	else
		cout<<"\nInvalid input.\n"<<endl;
}

[blackcoder41]

i suggest making a class that represent a fraction, here's a pseudo code

class fraction {
private:
  int numerator;
  int denominator;
public:
  fraction(int n, int d); // this is the constructor
  add();
  subtract();
  divide();
  multiply();
}

[helios]

I would suggest using overloaded operators if you're gonna use a class.

Oh, and a few other things:
* Functions that perform the lowest level operations (in this case, *Function()) should not perform I/O. I/O should only be performed in functions very close to the highest level, or through specialized wrapper functions that are called from very close to the highest level.
* menuFunction() has too much responsibility.

[blackcoder41]

totally agree with helios. so here's how it looks now.

class fraction {
private:
  int numerator;
  int denominator;
public:
  fraction(int n, int d); // this is the constructor
  operator + ();
  operator - ();
  operator * ();
  operator / ();
}

[yoked88]

Thanks for the suggestions guys. I haven't gotten to classes or operator overloading yet, I just wanted to pass lots of parameters to make sure I'm getting the hang of this. I do see what you both are trying to say though. I also tried putting the if statement that tests whether the denominators are zero in mainFunction, but for some reason it didn't work.

[jsmith]

That's OK, because blackcoder41's example code above is syntactically wrong anyway.

What is your question?

[blackcoder41]

@jsmith
can you please correct me, i'd love to learn from you.. thanks.

[helios]

fraction operator/*...*/(const fraction &);

[blackcoder41]

that's what's on my head. i forgot to say that i did not post the actual code.. lol.. thanks anyway.

[Disch]

even better:

fraction operator /*...*/ (const fraction&) const;

[jsmith]

Except for +=, -=, *=, and /= (and %= too, if it were relevant to fractions)

const fraction& operator+=( const fraction& rhs );

And, when needed, for exception safety, the best way to declare the assignment
operator (not needed for above class) is

const fraction& operator=( fraction rhs );

[blackcoder41]

C++ Operator Overloading Guidelines
http://www.cs.caltech.edu/courses/cs11/material/cpp/donnie/cpp-ops.html

[jsmith]

Apparently C++ allows this:

int x = 4;
int y = 5;

( x += y )++;

so I guess to be compatible, my above should be

fraction& operator+=( const fraction& rhs );

though why one would want to do that is beyond me.

[jsmith]

@blackcoder41

That page is out of date / wrong when it comes to the assignment operator.
It is recommended to implement assignment via copy/swap for exception safety:

// Of course the default assignment would work for this class, but to illustrate:
class MyClass {
    int x;
    std::string s;
  public:
    const MyClass& operator=( MyClass rhs ) {
        std::swap( x, rhs.x );
        std::swap( s, rhs.s );
        return *this;
   }
};

Checking for self-assignment is unnecessary (you can't even do it) since the rhs got
copied onto the stack.

[blackcoder41]

ok thanks.