Aero
Aero

Reputation: 73

Overriding = operator in C++

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

Answers (8)

Winston Ewert
Winston Ewert

Reputation: 45059

You want this:

Vec3 & Vec3::operator =(const Point &a) 
{
x = a.x;
y = a.y;
z = a.z;
return *this;
}
  1. Assignment should modify the this object, not return something
  2. Return a reference to object just modified

Upvotes: 2

Potatoswatter
Potatoswatter

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

Mike Seymour
Mike Seymour

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

ArBR
ArBR

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

Ben Jackson
Ben Jackson

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

J. Polfer
J. Polfer

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

Russell Borogove
Russell Borogove

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

Keynslug
Keynslug

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

Related Questions