C++ operator= overload works selectively

Question

I appear to have an assignment operator that works circumstatially. I have a Vector2 class which contains two float's, x and y. Here is the operator= method:

Vector2 Vector2::operator=(const Vector2 &right)
{
    Vector2 result;
    result.x = right.x;
    result.y = right.y;
    return result;
}

Within an onMove method for an entity I have:

Vector2 newPosition = position + (speed * getSpeedFactor());

if (posValid(newPosition))
{
    position.x = newPosition.x;
    position.y = newPosition.y;
    //position = newPosition;
}

Where speedFactor depends on framerate and posValid is a check for whether a point is in a wall or something. position and speed are also Vector2's. The first assignment:

Vector2 newPosition = position + (speed * getSpeedFactor());

works, and with the code as is I get the expected/intended behaviour, however

position = newPosition;

has no effect, whether on it's own or before or after the .x & .y assignments.

The first one is **NOT** the assignment operator, it's the constructor. Also, you should be returning `Vector2 &`. It's also being called on an object, so you should be modifying the object, not a temporary result. — chris, Jun 22 '12 at 14:27
Take a look at this: http://stackoverflow.com/questions/4421706/operator-overloading — chris, Jun 22 '12 at 14:29

Attila · Answer 1 · 2012-06-22T14:36:29.953

14

Your operator=() is wrong, it should modify the current object, not the extra you create

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

Note: the return type is a reference and you are returning the reference to the current object, not a copy of it (or of a new one like in your code).

The reason it worked for

Vector2 newPosition = ...;

is because this is not default-construct + assignment but a copy-construct call.

edited Jun 22 '12 at 14:36

answered Jun 22 '12 at 14:28

Attila

28,265
3
46
55

3

*"is because the compiler optimized away the default-construct+assignment to a copy-construct call"* -- It's not an optimization. When `=` is used in an initialization, it is not assignment, it is a call to the copy constructor. – Benjamin Lindley Jun 22 '12 at 14:34

score 1 · Answer 2 · answered Jun 22 '12 at 14:30

You aren't actually assigning to position. You are making a temporary Vector2 object, assigning to it, then implicitly copying it and returning the copy. You are never modifying the actual object.

You need something like:

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

C++ operator= overload works selectively

2 Answers2