Sign in to follow this  
guyver23

Operator Overloading in C++

Recommended Posts

guyver23    181
[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! :)
[/size]

Share this post


Link to post
Share on other sites
AdisH    122
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&.

Share this post


Link to post
Share on other sites
guyver23    181
[quote name='AdisH' timestamp='1307628152' post='4821313']
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&.
[/quote]

Thank you for the suggestion, but that does not appear to work...

Share this post


Link to post
Share on other sites
guyver23    181
[quote name='Hodgman' timestamp='1308024176' post='4823034']
What does your operator+ look like?
[/quote]

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]

Share this post


Link to post
Share on other sites
Hodgman    51336
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 ([i]meaning you've returned a reference to a variable that's been deleted, which is why you're printing garbage![/i])

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 [b]const[/b]-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) [b]const [/b]{...}[/font]

Read through these pages for clarification ([i]I'd recommend reading that entire FAQ when you've got the time[/i]):
[url="http://www.parashift.com/c++-faq-lite/references.html"]http://www.parashift...references.html[/url]
[url="http://www.parashift.com/c++-faq-lite/const-correctness.html"]http://www.parashift...orrectness.html[/url]

Share this post


Link to post
Share on other sites
guyver23    181
[quote name='Hodgman' timestamp='1308058166' post='4823206']
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 ([i]meaning you've returned a reference to a variable that's been deleted, which is why you're printing garbage![/i])

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 [b]const[/b]-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) [b]const [/b]{...}[/font]

Read through these pages for clarification ([i]I'd recommend reading that entire FAQ when you've got the time[/i]):
[url="http://www.parashift.com/c++-faq-lite/references.html"]http://www.parashift...references.html[/url]
[url="http://www.parashift.com/c++-faq-lite/const-correctness.html"]http://www.parashift...orrectness.html[/url]
[/quote]

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

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this