Insertion Operator

[stoneJax]

I have an overloaded << operator and it works with my fraction class. Now, I am adding some more code to it so it can satisfy the instructions as follows:

Printing a Fraction to a stream with an overloaded insertion (<<) operator. The Fraction should be printed in reduced form (not 3/6 but 1/2). Whole numbers should print without a denominator (e.g. not 3/1 but just 3). Improper Fractions should be printed as a mixed number with a + sign between the two parts (2+1/2). Negative Fractions should be printed with a leading minus sign.

I know my simplify function works but I do not know how to call it with these circumstances. Also, I have started some code for changing improper fractions into mixed numbers and I am not sure what to do with it from here.

  std::ostream& operator<<(std::ostream& out, const Fraction& right)
	{
		Fraction wholeNumber, remainder;

		out << right.numerator << "/" << right.denominator; 
		simplify();

			if (right.numerator > right.denominator) {
				wholeNumber = right.numerator / right.denominator;
				remainder = right.numerator % right.denominator; // wholeNumber + remainder / denominator is a mixed number
			}
			return out;
	}

[ne555]

		out << right.numerator << "/" << right.denominator; 
		simplify();

1- ¿whom do you want to `simplify'?
2- you are printing before simplifying

[stoneJax]

I am under the impression that if a fraction object is created, say Fraction f1(3, 6); then if I cout << f1(3, 6); it should display 1/2. So was trying to call simplify from within the overloaded << function. The call is not correct though. Simplify() get a red squiggly line that says identifier is undefined.

I'm not sure if I am approaching this right so sorry if it looks confusing.

[stoneJax]

I see what you mean by printing before simplifying. I changed it to:

simplify();
out << right.numerator << "/" << right.denominator; 

There is something wrong with the call though as it has the red squiggly line error.

[stoneJax]

I figured out the simplifying part. I had to get rid of const and use that parameter with the dot operator when calling simplify.

Now I am trying to define cout so an improper fraction will be printed as a mixed number. I get an unhandled exception in my constructor saying there is a stack overflow with the parameters.

Here is my updated code:

//in main
Fraction  f2(5, 2);
	cout << f2 << endl;
}





Fraction::Fraction(int inNumerator, int inDenominator)
	{
		assert(inDenominator != 0);
		numerator = inNumerator;
		denominator = inDenominator;
	}




std::ostream& operator<<(std::ostream& out, Fraction& right)
	{
		Fraction wholeNumber, remainder;  // int objects

		right.simplify();

		if (right.numerator > right.denominator) {

			wholeNumber = right.numerator / right.denominator;
			remainder = right.numerator % right.denominator;
			// wholeNumber + remainder / denominator is a mixed number
			out << wholeNumber << "+" << remainder << "/" << right.denominator;
		}
		else out << right.numerator << "/" << right.denominator; 
		
		return out;
	}

[stoneJax]

I think it is something with the numerator in the if() since it is not a parameter. I tried adding it as a parameter too, in both the prototype and definition, and I got red lines saying too many arguments. Not sure how to get the comparison right here?

[Peter87]

I figured out the simplifying part. I had to get rid of const and use that parameter with the dot operator when calling simplify.

Having to remove const make me suspicious, and it will prevent you from printing fractions that are returned from expressions directly.

Fraction  f1(5, 2);
Fraction  f2(5, 2);
std::cout << (f1 + f2); // error, if you have removed const from operator<< 

Printing something should normally not modify it. If the fraction 1/2 is totally equivalent to 2/4 and there is no way to tell the difference I would instead move the simplify function to the constructor and always make sure the fraction is stored in its simplified form.

Otherwise you probably want to create a copy of the fraction inside operator<< that you can simplify and print.

I get an unhandled exception in my constructor saying there is a stack overflow with the parameters.

The reason is probably because you have declared wholeNumber and remainder as fractions (aren't they supposed to be integers?) and then you printing them. This will lead to your operator<< calling itself until it eventually runs out of stack space (which causes a stack overflow).

[stoneJax]

What your saying with the constructor and simplify() makes a lot of sense. I tried organizing it that way and now my simplify function is getting an unhandled exception saying integer division by zero. I have tested this function with a lot of fractions and it has always worked before.

The fraction object I now tested with from main is 3/6.

Fraction::Fraction(int inNumerator, int inDenominator)
	{
		assert(inDenominator != 0);
		numerator = inNumerator;
		denominator = inDenominator;
		simplify();
	}




void Fraction::simplify()
	{
		int reducedNumerator = numerator;
		int reducedDenominator = denominator;

		for (int divisor = 1; divisor <= reducedNumerator &&
			divisor <= reducedDenominator; divisor++) {

			if (reducedNumerator % divisor == 0 && reducedDenominator % divisor == 0) {
				reducedNumerator = reducedNumerator / divisor;
				reducedDenominator = reducedDenominator / divisor;
			}
		}
		if ((numerator / reducedNumerator) == (denominator / reducedDenominator)) { // Error int division by zero
			numerator = reducedNumerator;
			denominator = reducedDenominator;
		}
	}

[stoneJax]

As for the data type of wholeNumber and remainder, thank you for pointing that out. I changed their data types to int and now the << operator converts improper fractions to mixed. I had thought that my private data members for numerator and denominator were ints that declaring fraction types would work but obviously not.

As for placing a simplify() call in the constructor changing the behavior of the simplify function I don't understand. If I remove that and call simplify from other functions that perform +,-,*,/ then it works.

[Peter87]

I didn't say anything about modifying the simplify function. The reason I said you should call simplify from the constructor was because I thought that might be enough, assuming all operators use the constructor to initialize the resulting fraction rather than assigning directly to the variables.

// E.g.
Fraction Fraction::operator+(const Fraction& f1, const Fraction& f2)
{
	return Fraction(f1.numerator + f2.numerator, f1.denominator + f2.denominator);
}

Be a bit careful with this:

if ((numerator / reducedNumerator) == (denominator / reducedDenominator)) { // Error int division by zero 

I don't see 3/6 leading to a division by zero here, but it could happen if the numerator is zero (e.g. 0/1)

[ne555]

your simplify() function is incorrect, tested with 4/16 returns 2/8 instead of 1/4
also, it will not handle negative fractions correctly.

> Having to remove const make me suspicious, and it will prevent you from printing fractions that are returned from expressions directly.
> Otherwise you probably want to create a copy of the fraction inside operator<< that you can simplify and print.
or pass the parameter by copy std::ostream& operator<<(std::ostream& out, Fraction right)

[dhayden]

return Fraction(f1.numerator + f2.numerator, f1.denominator + f2.denominator); That's not how you add fractions.

To simplify fractions, you want to divide the numerator and denominator by their greatest common divisor. You should also handle negative values: if the denominator is negative, switch the sign of numerator and denominator. If numerator is now negative, be sure to switch its sign before computing the GCD.

If you can guarantee that fractions are always stored in reduced form then the printing is pretty easy. Don't be afraid to do it recursively. In pseudocode:

if (numerator < 0 {
    print -fraction;
} else if (numerator > denominator) {
    print numerator/denominator
    print "+"
    print Fraction(numerator% denominator, denominator);
} else {
    print numerator "/" denominator
}

[Peter87]

return Fraction(f1.numerator + f2.numerator, f1.denominator + f2.denominator); That's not how you add fractions.

You're right. I should have picked the * operator as an example instead..

[stoneJax]

Peter87 I think you are right in saying, "I don't see 3/6 leading to a division by zero here, but it could happen if the numerator is zero (e.g. 0/1)." My constructor has default values of 0/1 and when I place a call to simplify() from the constructor is when I get the error saying integer division by zero. When simplify is called from any of the other functions with the testing that I have done the numerator hasn't been zero. The thing that is confusing though is integer division BY zero would mean the denominator so I don't understand why I get the error with the numerator. Could you please explain? Also, how am I to correct an error like this? My requirements are to have the constructors default values as 0/1.

[Peter87]

What is the purpose of that if statement? Maybe you can check the same thing without doing the division.

[stoneJax]

I commented out the last if() that is throwing the error and placed some cout statements in lines 24 and 25 for reducedNumerator and reducedDenominator. The output is as follows:

reducedNumerator: 9
reducedDenominator: 8
reducedNumerator: 2
reducedDenominator: 3

0/1

Given the constructors default value of 0/1 and the output above we can interpret the last if() as if((0/2) == (1/3)). This evaluates to 0 == 0 since the int decimal is truncated. Then the numerator and denominator get assigned the values which would be 0/0. So, I added an if statement to add one to the denominator if this is the case; however, I still get the same error of division by zero. I can't make any sense of why this error is here.

Updated code.

if ((numerator / reducedNumerator) == (denominator / reducedDenominator)) { // Error int division by zero
				numerator = reducedNumerator;
				denominator = reducedDenominator;

				if (denominator == 0)
					denominator += 1;
			}

[stoneJax]

Sorry I missed your post earlier. Must have needed a refresh. The purpose of that if statement is to test if they have the same greatest common divisor. I am not sure how to check that another way. At least not with how my code is already set up.

[Peter87]

But don't you already know that?

[stoneJax]

I think I figured it out. I created a variable to add the number of prime numbers that both num and denom have. Then replaced the if that was giving me problems with a similar if and used multiplication instead of division.

I can't see why the code I had is not working though. I kept trying to make what I have work. Otherwise I probably would have looked another way earlier. Thank you for pointing out to look in an alternative direction though.

[Peter87]

I still don't understand why you need to check anything after the loop at all. Why not always update numerator and denominator? If you do that you wouldn't even need the reducedNumerator and reducedDenominator because you could just let the loop modify numerator and denominator directly.