Reputation: 73
I am trying to override the = operator so that I can change my Point class into a Vector3 class.
Point tp = p2 - p1;
Vec3 v;
v = tp;
The problem I am facing is that, "v" will have its x,y,z members equal to zero all the time.
Vec3.h:
Vec3 operator =(Point a) const;
Vec3.cpp:
Vec3 Vec3::operator =(Point a) const
{
return Vec3(a.x,a.y,a.z);
}
Thanks for all the help once again :)
Upvotes: 5
Views: 29634
Reputation: 45059
You want this:
Vec3 & Vec3::operator =(const Point &a)
{
x = a.x;
y = a.y;
z = a.z;
return *this;
}
Upvotes: 2
Reputation: 137900
Hmm, 6 answers here already and you've already selected one, but the simplest and most obvious IMHO is missing.
Vec3 &Vec3::operator =(Point a) // not const, return by reference
{
return *this = Vec3(a.x,a.y,a.z);
}
That's all! Just insert *this =
and return by reference! Done!
I would recommend implementing a conversion constructor instead, though.
Upvotes: 0
Reputation: 254691
It's more common to do this with a conversion constructor:
Vec3(const Point& p) : x(p.x), y(p.y), z(p.z) {}
This will also allow the assignment that you want.
Upvotes: 1
Reputation: 4082
I agree with sheepsimulator in the fact that copy assignment operator should have the same behavior than copy constructor has. According with HIGH·INTEGRITY C++ CODING STANDARD MANUAL, you should implement an explicit conversion operator:
class Point { explicit operator Vec3() { return Vec3(this->x,this->y,this->z); } };
Upvotes: 1
Reputation: 93880
There are some answers explaining how to do what you want, but I will explain what's going on with your original code:
In C++ (as in C) your assignment v = tp
is an expression, not a statement. If you write x = 1
then you can write y = (x = 1)
because the assignment (x = 1)
is an expression with the value 1. When you override the assignment operator in C++, you are responsible for both parts: You must modify this
and you must return the value of the assignment expression. Your original version only took care of returning the value of the expression. If you had written Vec3 w = (v = tp)
then w would have had the expected value, while v would still be all 0.
The other way you can express the same thing in C++ is to add an operator Vec3
to your Point class so that you can use a Point anywhere you could use a Vec3. Even better is probably to have one class Vec3 and typedef Vec3 Point
because they are largely identical.
Upvotes: 0
Reputation: 12481
Why not just do this:
void Vec3::SetCoord(const Point& pnt)
{
x = pnt.x;
y = pnt.y;
z = pnt.z;
}
OR...
Make a new overloaded constructor:
Vec3::Vec3(const Point& pnt)
{
x = pnt.x;
y = pnt.y;
z = pnt.z;
}
No crazy casting or equals operator involved. I'd recommend doing one of these ways, its probably the easiest to maintain.
To get the equals-syntax above should really add a type conversion operator, but to Point:
class Point
{
// ....
operator Vec3()
{
return Vec3(this->x,this->y,this->z);
}
};
Doing an assignment between the two objects like that is kinda funny without assuming a type-conversion. Most classes don't behave that way syntactically.
Note, though, that this can cause some ambiguity, and a number of well-respected folks on this site who answer questions on C++ would say the type-conversion answer can cause all sorts of nasty issues. So I'd avoid it.
Upvotes: 0
Reputation: 19057
It's been a while, but I think you want
Vec3& Vec3::operator=(const Point &a)
{
x = a.x; y = a.y; z = a.z;
return *this; // Return a reference to myself.
}
Assignment modifies 'this', so it can't be const. It doesn't return a new Vec3, it modifies an existing one. You will also probably want a copy constructor from Point, that does the same.
Upvotes: 12
Reputation: 2766
Assignment operator work like this.
Vec3.h:
Vec3& operator = (const Point &a);
Vec3.cpp:
Vec3& Vec3::operator = (const Point &a)
{
x = a.x;
y = a.y;
z = a.z;
return *this;
}
Notice that you're modifying this
object and return a non-const reference to it.
Upvotes: 1