Operator Overloading in C++

Started by
5 comments, last by guyver23 12 years, 10 months ago
[size="4"]As an exercise, I decided to write a class to model the mathematical concept of a vector. I added a friend function to the class that allows the vector to be printed neatly to standard output; the results are not what I would expect.

This prints garbage:
[source lang="cpp"]
Vector A(1,2,3);
Vector B(4,5,6);

std::cout << A + B << std::endl;
[/source]

...while this works as expected:
[source lang ="cpp"]
Vector A(1,2,3);
Vector B(4,5,6);
Vector C = A + B;

std::cout << C << std::endl;
[/source]

I have not provided the Vector class with a custom assignment operator or a copy constructor. Why doesn't the first form work? If it is relevant, here is the code:
[source lang="cpp"]
std::ostream& operator << (std::ostream& lhs, Vector& rhs)
{
lhs << "<" << rhs.v1() << "," << rhs.v2() << "," << rhs.v3() << ">";
return lhs;
}
[/source]

Thanks! :)
Advertisement
Well, A + B returns a temporary. You can't bind a non-const reference to a temporary. You can only bind a const reference, or an rvalue reference (C++0x), so change your prototype to accept a const Vector&.

Well, A + B returns a temporary. You can't bind a non-const reference to a temporary. You can only bind a const reference, or an rvalue reference (C++0x), so change your prototype to accept a const Vector&.


Thank you for the suggestion, but that does not appear to work...
What does your operator+ look like?

What does your operator+ look like?


I apologize for having to asking such a basic question. Since I experience the same problem no matter which operator I use, I decided to post all my code. Thanks for your help!
[source lang="cpp"]
#include <cmath>
#include "Vector.h"

Vector::Vector() : i(0), j(0), k(0)
{
}

Vector::Vector(float x, float y, float z) : i(x), j(y), k(z)
{
}

float Vector::v1()
{
return this->i;
}

float Vector::v2()
{
return this->j;
}

float Vector::v3()
{
return this->k;
}

float Vector::magnitude()
{
return std::sqrt(i*i + j*j + k*k);
}

Vector& Vector::operator +(Vector& rhs)
{
float x, y, z;
x = this->v1() + rhs.v1();
y = this->v2() + rhs.v2();
z = this->v3() + rhs.v3();

Vector answer(x,y,z);
return answer;
}

Vector& Vector::operator -(Vector& rhs)
{
float x, y, z;
x = this->v1() - rhs.v1();
y = this->v2() - rhs.v2();
z = this->v3() - rhs.v3();

Vector answer(x,y,z);
return answer;
}

float Vector::operator *(Vector& rhs)
{
float answer = this->v1() * rhs.v1() + this->v2() * rhs.v2() + this->v3() * rhs.v3();
return answer;
}

std::ostream& operator << (std::ostream& lhs, Vector& rhs)
{
lhs << "<" << rhs.v1() << "," << rhs.v2() << "," << rhs.v3() << ">";
return lhs;
}
[/source]
In your operators, you're returning a reference to a temporary (that's bad!)

"[font="Courier New"]Vector answer[/font]" is a local variable created on the stack -- it's lifetime is that of the function that it's created inside. As soon as you return, that variable no longer exists -- but you're returning a reference to it (meaning you've returned a reference to a variable that's been deleted, which is why you're printing garbage!)

Your return types should just be "[font="Courier New"]Vector[/font]" instead of "[font="Courier New"]Vector&[/font]".

Also, as AdisH said earlier, your "[font="Courier New"]operator <<[/font]" (and all your other operators) should take a const-reference ("[font="Courier New"]const Vector& rhs[/font]").
The member-functions themselves should be const too - e.g.
[font="Courier New"]Vector Vector::operator +(const Vector& rhs) const {...}[/font]

Read through these pages for clarification (I'd recommend reading that entire FAQ when you've got the time):
http://www.parashift...references.html
http://www.parashift...orrectness.html

In your operators, you're returning a reference to a temporary (that's bad!)

"[font="Courier New"]Vector answer[/font]" is a local variable created on the stack -- it's lifetime is that of the function that it's created inside. As soon as you return, that variable no longer exists -- but you're returning a reference to it (meaning you've returned a reference to a variable that's been deleted, which is why you're printing garbage!)

Your return types should just be "[font="Courier New"]Vector[/font]" instead of "[font="Courier New"]Vector&[/font]".

Also, as AdisH said earlier, your "[font="Courier New"]operator <<[/font]" (and all your other operators) should take a const-reference ("[font="Courier New"]const Vector& rhs[/font]").
The member-functions themselves should be const too - e.g.
[font="Courier New"]Vector Vector::operator +(const Vector& rhs) const {...}[/font]

Read through these pages for clarification (I'd recommend reading that entire FAQ when you've got the time):
http://www.parashift...references.html
http://www.parashift...orrectness.html


Thank you very much for explaining my mistakes! :wink:

This topic is closed to new replies.

Advertisement