C++ Copy function overload results in "must be a nonstatic member function" error

Question

I am having an error when I try to compile my code. The code is overloading operators and all of the overloaded operators have worked until I tried to assign the copy constructor. I keep getting a "MyClass operator=(const MyClass&)’ must be a nonstatic member function" error. I don't understand why overloading the "=" operator would cause such an error. Putting the "friend" special word in front of the declaration in the .h file does not fix the issue.

main.cpp

#include <iostream> #include "Point.h" using namespace std; int main() { Point point1( 2, 5); Point point2 = point1; return 0;

}

MyClass.h

 #ifndef POINT_H_ #define POINT_H_ #include <iostream> #include <cmath> using namespace std; class Point { public: //Constructor Point(); Point(const double x, const double y); //Copy Point(const Point & t); //Destructor virtual ~Point(); //Get the x value double getX() const; //Get the y value double getY() const; //Set the x value void setX(double x); //Set the y value void setY(double y); //Return the distance between Points double distance(const Point& p) const; //Output the Point as (x, y) to an output stream friend ostream& operator << (ostream& out, const Point& point); //Comparison relationships friend bool operator == (const Point& lhs, const Point& rhs); friend bool operator < (const Point& lhs, const Point& rhs); //Math operators friend Point operator + (const Point& lhs, const Point& rhs); friend Point operator - (const Point& lhs, const Point& rhs); Point& operator = (const Point& rhs); private: double x; double y; }; #endif /* POINT_H_ */

MyClass.cpp

 // Get the x value double Point::getX() const { return x; } // Get the y value double Point::getY() const { return y; } void Point::setX(double x) { this->x = x; } void Point::setY(double y) { this->y = y; } // Return the distance between Points double Point::distance(const Point& p) const{ return abs( sqrt( pow( (x - p.getX() ), 2 ) + pow( (y - p.getY() ), 2 ) ) ); } ostream& operator << (ostream& out, const Point& point){ out << point.getX() << ", " << point.getY(); return out; } bool operator == (const Point& lhs, const Point& rhs){ if(lhs.x == rhs.x && lhs.y == rhs.y){ return true; } return false; } bool operator < (const Point& lhs, const Point& rhs){ if(lhs.x < rhs.x && lhs.y < rhs.y){ return true; } return false; } Point operator + (const Point& lhs, const Point& rhs){ Point point; point.x = lhs.x + rhs.x; point.y = lhs.y + rhs.y; return point; } Point operator - (const Point& lhs, const Point& rhs){ Point point; point.x = lhs.x - rhs.x; point.y = lhs.y - rhs.y; return point; } Point& Point::operator = (const Point& rhs){ x = rhs.x; y = rhs.y; return *this; } // Destructor Point::~Point(){}

But the code has other problems, too. Your favourite compiler will tell you. — Swordfish
– Swordfish, Commented Sep 30, 2018 at 5:47
That worked now I am getting the warning "reference to local variable ‘MyClass’ returned [-Wreturn-local-addr]" with the updated code above. — TheWinterSnow
– TheWinterSnow, Commented Sep 30, 2018 at 6:00

alter_igel · Accepted Answer · 2018-09-30 06:40:07Z

There are a number of problems here that I'll try to break down, one by one.

With regard to your main question, because the copy assignment operator is defined inside the scope of MyClass, you also need to write its definition as being scoped inside MyClass, simply by adding MyClass:: before the function's name, as you would with any other scope. Next, you really should actually copy the data of rhs into this, and you should return a reference to *this.

MyClass& MyClass::operator=(const MyClass& rhs){ x = rhs.x; y = rhs.y; return *this; }

If you copy rhs into a temporary and return that, the whole semantics of copy assignment is completely broken. It'll lead to things like this:

MyClass obj1 {999.0, 2468.0}; MyClass obj2 {0.0, 0.0}; obj2 = obj1; // should copy obj1 to obj2 cout << obj2.x << " " << obj2.y << '\n'; // outputs "0.0 0.0" ??!?!?

Also be aware that this is a trivial copy constructor, you can remove it completely that the compiler will implicitly generate an identical one for you that does a member-wise copy, which in this case is completely safe. See special member functions.

In your operator+ function body, there are these lines:

MyClass myVar; MyClass.x = lhs.x + rhs.x; MyClass.y = lhs.y + rhs.y; return myVar;

You probably mean myVar.x and myVar.y. MyClass only refers to a type and trying to access MyClass.x is a meaningless statement.

As a rule, you should never return a reference to a temporary object as in your current implementation. Local objects get destroyed when the function returns and this leaves you with a dangling reference to a destroyed object, whose memory will very likely be overwritten with complete garbage in the near future. Here is a good explanation of what's going on.

Let me know if you run into any further problems or if something isn't clear after reading the links. Happy coding!

The operator + code was incorrectly translated from my original code as I mentioned before it does work. The problem that I have now is that the = operator needs to make a new copy from one object variable to another.
@TheWinterSnow can you explain a bit more? A copy assignment operator that doesn't assign to this doesn't make sense. It might help to carefully read here, though the examples are a bit complicated. The first two headings should help.
@TheWinterSnow the update doesn't help much. Is Point supposed to be the same as MyClass? Please explain what you mean by "the = operator needs to make a new copy." The copy assignment operator doesn't make any new objects, it should always leave one object logically identical to another.
the = operator is supossed to copy the contents of the class to another class. It should not copy the pointer or reference to the other variable.

Collectives™ on Stack Overflow

C++ Copy function overload results in "must be a nonstatic member function" error

1 Answer 1

11 Comments

Hot Network Questions