Jump to content
  • Advertisement
Sign in to follow this  
guyver23

Operator Overloading in C++

This topic is 2538 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

[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! :)

Share this post


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

Share this post


Link to post
Share on other sites

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...

Share this post


Link to post
Share on other sites

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]

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites

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:

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!