CODEWITHSUNDEEP
c++pointersoperator-overloading
user1966471
user1966471

Reputation:

Pointer to class

I have been trying to write a Fraction class and overload some of its operators (+, -, -, /...). At first, I tried to do it like this:

Fraction& operator+(Fraction& rightOp)
{
    Fraction result;
    result.num = num * rightOp.den + den * rightOp.num;
    result.den = den * rightOp.den;;
    return result;
}

This produced an awkward result. While testing, if I used:

Fraction a(2,3);
Fraction b(4,5);
Fraction c = a + b;
cout << c << endl;

It would print correctly. However, if I used:

Fraction a(2,3);
Fraction b(4,5);
Fraction c;
c = a + b;
cout << c << endl;

It would print -858993460/-858993460.

Then, when I tried to change the overloading function to:

Fraction& operator+(Fraction& rightOp)
{
    Fraction* result = new Fraction;
    result->num = num * rightOp.den + den * rightOp.num;
    result->den = den * rightOp.den;
    return *result;
}

It would all work well. This got me confused about pointing to classes in C++, I do not really understand why the first one fails in a specific case. I would appreciate any explanation.

Thanks in advance.

Note: Operator << is not the source of the problem but here it is anyway:

friend ostream& operator<<(ostream& out, Fraction& f)
{
    out << f.num << "/" << f.den << endl;
    return out;
}

Upvotes: 0

Views: 119

Answers (2)

David G
David G

Reputation: 96810

Your + overload returns an instance of Fraction by-reference. The problem is that the local instance of Fraction will go out of scope at the end of the expression a + b, but c will still hold a reference to that empty spot in memory.

Return by-reference is not what you want here. It's more suitable to make a copy in this case by returning by-value:

Fraction operator +(Fraction& rightOp)
{
    Fraction result;
    result.num = num * rightOp.den + den * rightOp.num;
    result.den = den * rightOp.den;;
    return result;
}

Moreover, it's recommended that your >> overload take a Fraction instance by const reference, since it does not modify it inside the body:

friend ostream& operator<<(ostream& out, Fraction const& f)
//                                                ^^^^^

Upvotes: 2

Joseph Mansfield
Joseph Mansfield

Reputation: 110658

Fraction& operator+(Fraction& rightOp)
{
    Fraction result;
    result.num = num * rightOp.den + den * rightOp.num;
    result.den = den * rightOp.den;;
    return result;
}

The problem with this is that it's returning a reference to a local variable. The local variable is destroyed when the function ends, so the reference refers to an invalid object.

Using new instead has solved the problem for you because it makes the result object dynamically allocated. Such an object is not destroyed at the end of the function. However, it is certainly not the solution here. It is completely unclear to the user of operator+ that the reference returned refers to a dynamically allocated object and needs to be deleted in the future. Never put that burden on your users.

Instead, you should just change the function so that it returns by value:

Fraction operator+(Fraction& rightOp)
{
  // ...
  return result;
}

This will now return a copy of result, so it doesn't matter if result is destroyed at the end of the function.

As an additional note, you probably want the function to take a const Fraction& argument. This allows it to be called when the second operand is an rvalue (typically a temporary object).

Upvotes: 4

Related Questions