Overloaded operator is never called in C++

Question

I'm writing a math library as a practical exercise. I've run into some problems when overloading the = operator. When I debuged it, I noticed that the call to vertex1 = vertex2 calls the copy constructor instead.

In the header file I have:

//constructors
vector3();
vector3( vector3 &v );
vector3(float ix, float iy, float iz);

//operator overloading
vector3 operator =(vector3 p);
....

In the source file I implemented:

vector3 vector3::operator =(vector3 p)
{
    vector3 v3;
    v3.x = p.x;
    v3.y = p.y;
    v3.z = p.z;
    return v3;
}

Later on I have a crossproduct method, and I want to use it like so:

vector3 v3;
v3 = v1.crossProduct(v2);

The error message is: error: no matching function for call to `vector3::vector3(vector3)' but I do not want to call the copy constructor.

Answer 1

There are mistakes in your code. Your copy-constructor must take a const&. The reference will avoid making a copy (which you wouldn't be able to do, being the copy-constructor), and it should be const since you're not modifying it:

vector3(const vector3&);

Temporary variables can be bound to const&, but cannot be bound to a mutable reference. That is, with your code you could do:

vector3 a;
vector3 b(a);

but not:

vector3 a(some_calculation()); // some_calculation returns a vector3

Additionally, your operator= is incorrect. Like the copy-constructor, it should generally take a const&, but it should return a reference to this. That's how chaining works:

int a, b, c;
a = b = c = 0;
// a.operator=(b.operator=(c.operator=(0)));

Returning a temporary is unorthodox, and doesn't accomplish anything. In your case, you could assign over and over and never change the value. Weird:

vector 3 a, b;
a = b; // doesn't change a...?!

operator= needs to change this.

Answer 2

vector3( vector3 &v );

That really should be vector3( const vector3 &v );

Since your return a temporary value, you must call a copy-constructor which takes a const reference.

Answer 3

I do not want to call the copy constructor.

What you want is irrelevant. You need a copy constructor here. operator = doesn’t get called in this situation, the copy constructor is. Besides, the signature is wrong, it should be

vector3& operator =(vector3 const& other);

The argument may also be passed in by value (but this is an advanced trick …) but the return value really must be a non-const reference.

(The signature of your copy constructor is also unconventional, see James’ answer.)

Answer 4

Make vector3 vector3::operator =(vector3 p) use references instead so you don't need to create a copy.

vector3& vector3::operator =(vector3& p);

You didn't want to create a copied object in the first place anyway.

Answer 5

It is good practice in C++ to do one of two things depending on whether you want your object copyable (i.e assignable to another variable) or not. If you do you need to provide both the assign operator and the copy constructor. For example:

class Point
{
public:
    Point ()                          { }
    Point (int x, int y)              : mX(x), mY(y) { }
    Point (const Point& p)            : mX(p.mX), mY(p,mY) { }

    Point& operator = (const Point& p)    { mX = p.mX; mY = p.mY; return *this; }

    int X () const                    { return mX; }
    int Y () const                    { return mY; }

private:
    int mX;
    int mY;
};

If you don't want it copyable you can put the prototype of both the copy constructor and the assign operator in a private section and do not provide an implementation. Any attempt to copy it then will give a compiler error.

Whenever you use this kind of code:

Point P = anotherP;

the copy constructor will be called. If you use this type of code:

Point P;
P = anotherP;

the assign operator will be called.

Hope that helps.

Answer 6

When you "pass by value", as you are in your definition of operator =, a copy of the type is made for use a as local value to the method. Your operator isn't being called because the system cannot find a contructor that takes vector3 -- you've defined a copy constructor that takes vector3&.

Therefore, as others have stated, what you want to do is define your operator = as taking

const vector3& p

You should also update your declared copy constructor to take const vector3 also.