0

I was re-writing some code from a class I took a while ago and the original code was in eclipse CDT now I'm trying to use visual studio and that's where I re-wrote the code, but in visual studio it does not compute the assignment operator (=) well and puts garbage values while in eclipse it runs great and gives the Fraction final simplified answer. If you spot something wrong in my code please let me know. I was trying out the sum option of the program and the code breaks in the line f3 = f1 + f2; Example of run:

Code running in Eclipse CDT
Welcome to the fractions operations tutor
------------------------------------------------------
Option 1 - Sum of Fractions
Option 2 - Subtraction of Fractions
Option 3 - Multiplication of Fractions
Option 4 - Division de Fractions
Option 5 - Comparare Fractions
Option 6 - Exit
------------------------------------------------------------
1
Enter the numerator:2
Enter the denominator:3
Enter the numerator:3
Enter the denominator:3
2     3     5
-- + --   = --
3     3     3
Enter -1 if you want to exit this level,
If you wish to continue press a diferent number.

Run in vscode
Welcome to the fractions
-------------------------
Option 1 - Sum of Fractio
Option 2 - Subtraction of
Option 3 - Multiplication
Option 4 - Division de Fr
Option 5 - Comparare Frac
Option 6 - Exit
-------------------------
1
Enter the numerator:2
Enter the denominator:3
Enter the numerator:3
Enter the denominator:3
2     3     -858993460
-- + --   = --
3     3     -858993460

#include <iostream>
using namespace std;
class Fraction
{
private:
    int numerator, denominator;
    int GreatestCommonDenominator(Fraction& tempFraction);
public:
    Fraction(int = 1, int = 1);
    Fraction(const Fraction&);
    ~Fraction();
    void setFraction(int, int);
    void setNumerator(int);
    void setDenominator(int);
    int getNumerator() const;
    int getDenominator() const;
    void changeSign();
    void displayMenu(int& option);
    void exitDisplayMenu(int& out);
    void displayFraction() const;
    void sumTwoFractions(Fraction);
    void divideTwoFractions(Fraction);

    Fraction &operator=(const Fraction& tempFraction);
    Fraction &operator+(const Fraction& tempFraction);
    Fraction &operator-(const Fraction& tempFraction);
    Fraction& operator*(const Fraction& tempFraction);
    Fraction& operator/(const Fraction& tempFraction);

    bool operator <(const Fraction&);
    bool operator >(const Fraction&);
    bool operator ==(const Fraction&);
    bool operator !=(const Fraction&);

    friend istream& operator>>(istream&, Fraction&);
    friend ostream& operator<<(ostream&, const Fraction&);


};

fraction.cpp

#include "Fraction.h"

Fraction::Fraction(int tempNumerator, int tempDenominator) :numerator(tempNumerator), denominator(tempDenominator)
{
}

Fraction::Fraction(const Fraction& tempFraction)
{
    setNumerator(tempFraction.getNumerator());
    setDenominator(tempFraction.getDenominator());
}

Fraction::~Fraction() 
{
}

void Fraction::setFraction(int tempNumerator, int tempDenominator)
{
    setNumerator(tempNumerator);
    setDenominator(tempDenominator);
}

void Fraction::setNumerator(int tempNumerator)
{
    this->numerator = tempNumerator;
}

void Fraction::setDenominator(int tempDenominator)
{
    if (tempDenominator != 0)
    {
        this->denominator = tempDenominator;
    }
    else
    {
        while (tempDenominator == 0)
        {
            cout << "The denominator can't be zero. Re-enter:";
            cin >> tempDenominator;
            this->denominator = tempDenominator;
        }
    }
}

int Fraction::getNumerator() const
{
    return this->numerator;
}

int Fraction::getDenominator() const
{
    return this->denominator;
}

void Fraction::changeSign()
{
    setNumerator(-1 * getNumerator());
}

void Fraction::displayMenu(int& option)
{
    cout << "Welcome to the fractions operations tutor" << endl;
    cout << "------------------------------------------------------" << endl;
    cout << "Option 1 - Sum of Fractions" << endl;
    cout << "Option 2 - Subtraction of Fractions" << endl;
    cout << "Option 3 - Multiplication of Fractions" << endl;
    cout << "Option 4 - Division de Fractions" << endl;
    cout << "Option 5 - Comparare Fractions" << endl;
    cout << "Option 6 - Exit" << endl;
    cout << "------------------------------------------------------------" << endl;
    cin >> option;
}

void Fraction::exitDisplayMenu(int& out)
{
    cout << "Enter -1 if you want to exit this level,\n"
        << "If you wish to continue press a diferent number.";
    cin >> out;
}

void Fraction::displayFraction() const
{
    cout << getNumerator() << endl;
    cout << "--" << endl;
    cout << getDenominator() << endl;

}

void Fraction::sumTwoFractions(Fraction tempFraction)
{
    setNumerator(getNumerator() * tempFraction.getDenominator() + getDenominator() * tempFraction.getNumerator());
    setDenominator(getDenominator() * tempFraction.getDenominator());
}

void Fraction::divideTwoFractions(Fraction tempFraction)
{
    setNumerator(getNumerator() * tempFraction.getDenominator());
    setDenominator(getDenominator() * tempFraction.getNumerator());
}

int Fraction::GreatestCommonDenominator(Fraction& tempFraction)
{
    int gcd;
    int y;
    if (tempFraction.getNumerator() > tempFraction.getDenominator())
    {
        gcd = tempFraction.getNumerator();
        y = tempFraction.getDenominator();
    }
    else
    {
        gcd = tempFraction.getDenominator();
        y = tempFraction.getNumerator();
    }
    while (y != 0)
    {
        int r = gcd % y;
        gcd = y;
        y = r;
    }
    return gcd;
}

Fraction& Fraction::operator+(const Fraction& tempFraction)
{
    Fraction sum;
    sum.setNumerator((this->numerator * tempFraction.getDenominator()) + (this->denominator * tempFraction.getNumerator()));
    sum.setDenominator(this->denominator * tempFraction.getDenominator());
    int gcd = GreatestCommonDenominator(sum);
    sum.setNumerator(sum.numerator / gcd);
    sum.setDenominator(sum.denominator / gcd);
    return sum;
}

Fraction& Fraction::operator-(const Fraction& tempFraction)
{
    Fraction subtract;
    subtract.setNumerator((this->numerator * tempFraction.getDenominator()) - (this->denominator * tempFraction.getNumerator()));
    subtract.setDenominator(this->denominator * tempFraction.getDenominator());
    int gcd = GreatestCommonDenominator(subtract);
    subtract.setNumerator(subtract.numerator / gcd);
    subtract.setDenominator(subtract.denominator / gcd);
    return subtract;
}

Fraction& Fraction::operator *(const Fraction& tempFraccion) 
{
    Fraction multiply;
    multiply.setNumerator((this->numerator) * (tempFraccion.getNumerator()));
    multiply.setDenominator((this->denominator) * (tempFraccion.getDenominator()));
    int gcd = GreatestCommonDenominator(multiply);
    multiply.setNumerator(multiply.numerator / gcd);
    multiply.setDenominator(multiply.denominator / gcd);
    return multiply;
}

Fraction& Fraction::operator /(const Fraction& tempFraccion) 
{
    Fraction divide;
    divide.setNumerator((this->numerator) * (tempFraccion.getDenominator()));
    divide.setDenominator((this->denominator) * (tempFraccion.getNumerator()));
    int gcd = GreatestCommonDenominator(divide);
    divide.setNumerator(divide.numerator / gcd);
    divide.setDenominator(divide.denominator / gcd);
    return divide;
}


Fraction& Fraction::operator=(const Fraction& tempFraction)
{
    this->setNumerator(tempFraction.getNumerator());
    this->setDenominator(tempFraction.getDenominator());
    return *this;
}

bool Fraction::operator<(const Fraction& tempFraction)
{
    bool flag = false;
    if ((this->numerator * tempFraction.getDenominator()) < (this->denominator * tempFraction.getNumerator()))
        flag = true;
    return flag;
}

bool Fraction::operator>(const Fraction& tempFraction)
{
    bool flag = false;
    if ((this->numerator * tempFraction.getDenominator()) > (this->denominator * tempFraction.getNumerator()))
        flag = true;
    return flag;
}

bool Fraction::operator==(const Fraction& tempFraction)
{
    bool flag = false;
    if ((this->numerator * tempFraction.getDenominator()) == (this->denominator * tempFraction.getNumerator()))
        flag = true;
    return flag;
}

bool Fraction::operator!=(const Fraction& tempFraction)
{
    bool flag = false;
    if ((this->numerator * tempFraction.getDenominator()) != (this->denominator * tempFraction.getNumerator()))
        flag = true;
    return flag;
}

ostream& operator<<(ostream& output, const Fraction& tempFraction)
{
    output << "Numerator:" << tempFraction.numerator << endl;
    output << "\t\t--" << endl;
    output << "Denominator:" << tempFraction.denominator << endl;
    return output;
}

istream& operator>>(istream& input, Fraction& tempFraction)
{
    cout << "Enter the numerator:";
    input >> tempFraction.numerator;
    cout << "Enter the denominator:";
    input >> tempFraction.denominator;
    return input;
}

main.cpp

#include<iostream>
#include<iomanip>
#include"Fraction.h"

using namespace std;

int main()
{
    Fraction f1, f2, f3;
    int option;
    do
    {
        int exit = 0;
        Fraction f1, f2, f3;
        f1.displayMenu(option);
        switch (option)
        {
        case 1:
            while (exit != -1)
            {
                cin >> f1;
                cin >> f2;
                f3 = f1 + f2;

                cout << f1.getNumerator() << setw(5) << " " << setw(1) << f2.getNumerator() << setw(5) << " " << f3.getNumerator() << endl;
                cout << "--" << setw(1) << " + " << "-- " << setw(4) << " = " << setw(1) << "--" << endl;
                cout << f1.getDenominator() << setw(5) << " " << setw(1) << f2.getDenominator() << setw(5) << " " << f3.getDenominator() << endl;
                f1.exitDisplayMenu(exit);
            }
            break;
        case 2:
            while (exit != -1)
            {
                cin >> f1;
                cin >> f2;
                f3 = f1 - f2;
                cout << f1.getNumerator() << setw(5) << " " << setw(2) << f2.getNumerator() << setw(6) << " " << f3.getNumerator() << endl;
                cout << "-- " << setw(1) << " - " << "-- " << setw(4) << " = " << setw(1) << "--" << endl;
                cout << f1.getDenominator() << setw(5) << " " << setw(2) << f2.getDenominator() << " " << setw(6) << f3.getDenominator() << endl;
                f1.exitDisplayMenu(exit);
            }
            break;
        case 3:
            while (exit != -1) 
            {
                cin >> f1;
                cin >> f2;
                f3 = f1 * f2;
                cout << f1.getNumerator() << setw(5) << " " << setw(2) << f2.getNumerator() << setw(6) << " " << f3.getNumerator() << endl;
                cout << "-- " << setw(1) << " * " << "-- " << setw(4) << " = " << setw(1) << "--" << endl;
                cout << f1.getDenominator() << setw(5) << " " << setw(2) << f2.getDenominator() << " " << setw(6) << f3.getDenominator() << endl;
                f1.exitDisplayMenu(exit);
            }
            break;
        case 4:
            while (exit != -1) 
            {
                cin >> f1;
                cin >> f2;
                f3 = f1 / f2;
                cout << f1.getNumerator() << setw(5) << " " << setw(2) << f2.getNumerator() << setw(6) << " " << f3.getNumerator() << endl;
                cout << "-- " << setw(1) << " / " << "-- " << setw(4) << " = " << setw(1) << "--" << endl;
                cout << f1.getDenominator() << setw(5) << " " << setw(2) << f2.getDenominator() << " " << setw(6) << f3.getDenominator() << endl;
                f1.exitDisplayMenu(exit);
            }
            break;
        case 5:
            while (exit != -1) {
                cin >> f1;
                cin >> f2;
                if (f1 > f2) {
                    cout << f1.getNumerator() << setw(5) << " " << setw(2) << f2.getNumerator() << setw(6) << " " << endl;
                    cout << "-- " << setw(1) << " > " << "-- " << setw(1) << endl;
                    cout << f1.getDenominator() << setw(2) << " " << setw(5) << f2.getDenominator() << " " << setw(6) << endl;
                }
                else if (f1 < f2) {
                    cout << f1.getNumerator() << setw(5) << " " << setw(2) << f2.getNumerator() << setw(6) << " " << endl;
                    cout << "-- " << setw(1) << " < " << "-- " << setw(1) << endl;
                    cout << f1.getDenominator() << setw(2) << " " << setw(5) << f2.getDenominator() << " " << setw(6) << endl;

                }
                else if (f1 == f2) {
                    cout << f1.getNumerator() << setw(5) << " " << setw(2) << f2.getNumerator() << setw(6) << " " << endl;
                    cout << "-- " << setw(1) << " = " << "-- " << setw(1) << endl;
                    cout << f1.getDenominator() << setw(2) << " " << setw(5) << f2.getDenominator() << " " << setw(6) << endl;
                }
                else {
                    cout << f1.getNumerator() << setw(5) << " " << setw(2) << f2.getNumerator() << setw(6) << " " << endl;
                    cout << "-- " << setw(1) << " != " << "-- " << setw(1) << endl;
                    cout << f1.getDenominator() << setw(2) << " " << setw(5) << f2.getDenominator() << " " << setw(6) << endl;

                }
                f1.exitDisplayMenu(exit);
            }
            break;
        case 6: 
            break;
        default:
            cout << "Enter a number from 1-6: ";
            cin >> option;
        }
    } while (option != 6);

    return 0;
}
W.Felici
  • 1
  • 1
  • 2
    Please form a proper [repro]. That means removing all the parts of the code that are not necessary to reproduce the particular error. Also please provide the exact input and output from the run causing the wrong behavior. – walnut Nov 18 '19 at 00:32
  • 1
    Specific binary operators (ex: + - * / etc.) need to return values; not references. When used in the form `obj3 = obj1 op obj2;` , op needs to return by-value, not reference. You're literally building a result, then value returning that. Note that is *not* the case for operators that target one of the operands. Ex: += *= /= etc. – WhozCraig Nov 18 '19 at 00:58
  • 1
    See this question on [operator overloading](https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading). – 1201ProgramAlarm Nov 18 '19 at 01:34

2 Answers2

3

In Fraction& Fraction::operator+(const Fraction& tempFraction):

Fraction& Fraction::operator+(const Fraction& tempFraction)
{
    Fraction sum;
    sum.setNumerator((this->numerator * tempFraction.getDenominator()) + (this->denominator * tempFraction.getNumerator()));
    sum.setDenominator(this->denominator * tempFraction.getDenominator());
    int gcd = GreatestCommonDenominator(sum);
    sum.setNumerator(sum.numerator / gcd);
    sum.setDenominator(sum.denominator / gcd);
    return sum;
}

sum is a local variable.

...and you later returned it BY REFERENCE.

NEVER return a local variable by reference. It is destroyed after the function returns, and any reference to it becomes a dangling reference.

You should return a Fraction rather than a Fraction&. Same applies to all of your other -, *, / operators.

And please do not ignore compiler warnings. In your case you should get a C4172: returning address of local variable or temporary: sum from MSVC.

ph3rin
  • 4,426
  • 1
  • 18
  • 42
2

Well, can't see exactly why, but i can see an error on your operator declaration... you are returning the l-value of a object that is inside the stack and you should never do it: instead you should return the object, an c++ will take care of assigning a copy, by using the copy constructor, of that object to the left element of the assignment, so switch every Fraction& operator... to Fraction operator... instead, in the sumTwoFractions and in the divideTwoFractions functions, the object you are passing an object, instead of an alias of that, so useless object is been created (but this won't cause issues)

Also :

  • In your main function, you are redeclaring Fraction f1, f2, f3; in the do-while loop
  • exitDisplayMenu and displayMenu should be static from the moment that they don't depend on the object from where those methods are invoked
  • the destroyer from the moment the is totally empty, you can avoid to explicit it, but if you prefer to be explicit, just use Fraction::~Fraction()= default;
Alberto Sinigaglia
  • 12,097
  • 2
  • 20
  • 48