CODEWITHSUNDEEP
c++visual-c++operator-overloading
Synaesthesics
Synaesthesics

Reputation: 1

Operator Overloading and LNK2019 error in VS2010

I'm writing a Point class which holds three floats, x y z, some functions and overloaded operators. I coded operators in the following form:

inline Point Point::operator+ (Point point)
{
    return Point(x + point.x, y + point.y, z + point.z);
}

inline void Point::operator+= (Point point)
{
   x += point.x;
   y += point.y;
   z += point.z;
}

Is this the right way to overload these operators? I've tested it and it works but I've seen another form like so:

inline Point& Point::operator+ (Point& point)
{
    return Point(x + point.x, y + point.y, z + point.z);
}

inline Point& Point::operator+= (Point& point)
{
    x += point.x;
    y += point.y;
    z += point.z;
        return *this;
}

What is the difference between the two forms?

Also I can use the operators inside my Point.cpp file but if I try to use it say in Main.cpp, I get a lnk2019 error for unresolved external symbol. Oddly, my functions work outside the defining file. What am I missing to get these operators to work outside the file they're defined in?

Upvotes: 0

Views: 1038

Answers (2)

K-ballo
K-ballo

Reputation: 81349

inline Point& Point::operator+ (Point& point)
{
    return Point(x + point.x, y + point.y, z + point.z);
}

That's wrong, its returning a reference to a temporary that no longer exists when the function returns. It would result in a segfault if you are lucky enough. Most compilers will give you a warning if you try to do it. Ideally you would write this operator as a free function, implemented in terms of your member operator+=.

inline Point& Point::operator+= (Point& point)
{
    x += point.x;
    y += point.y;
    z += point.z;
        return *this;
}

This is pretty much the preferred way, except that you should be taking the point as a const reference, otherwise you cannot use it with temporaries. Returning a reference to itself is what allows chaining operators. It's generally said that when in doubt, do as the ints, and the ints do that.

Summing up, the 'cannonical' implementation would be:

inline Point& Point::operator+=( Point const& point )
{
    x += point.x;
    y += point.y;
    z += point.z;
    return *this;
}

inline Point const operator+( Point left, Point const& right )
{
     return left += right;
}

Note that the having operator+ be a free function allows conversions to Point to happen at both operands and not just the right one. It's conveniently implemented in terms of operator+=: The left argument is taken by value in order to have a temporary copy that we can modify, and we do so by adding the right argument to it. Since operator+= returns a reference, we use such return in order to provide the value that would be copied as the function result.

Upvotes: 2

Daniel
Daniel

Reputation: 31579

The first operator should be

inline Point Point::operator+ (const Point &point) const
{
    return Point(x + point.x, y + point.y, z + point.z);
}

it shouldn't return a reference because its excepted to construct a new object and not modify a existing one (a general rule of thumb). const is added to the function because the point its called one shouldn't be modified. a reference to the point as argument is added so there won't be unnecessary copy

The second one should be 

inline Point& Point::operator+= (const Point& point)
{
    x += point.x;
    y += point.y;
    z += point.z;
    return *this;
}

because it supposed to modify an existing point, so it should return a reference. const is added to the argument because it shouldn't be changed. the function itself is not const because it should modify the point itself.

The linker error you get is because of the inline. either provide full implementation in header file, or remove the inline.

Upvotes: 3

Related Questions