Brandon
Brandon

Reputation: 23498

Copy/Move Assignment Confusion

In a move assignment operator, should I be using std::move or std::swap?

I did:

void Swap(Image& Img) {/*Swap everything here*/}

Image& Image::operator = (const Image &Img)
{
    if (this != &Img) Img.Swap(*this); //Compiler error for copy assignment.. no match for swap taking const..
    return *this;
}

Image& Image::operator = (Image&& Img)
{
    Img.Swap(*this);
    return *this;
}

Assume I have two Images I and J and I do:

I = std::move(J);

What happens is that the data from I and J are swapped so now J has the pixels of I and vice-versa. Is this normal? I thought move assignment was supposed to steal and destroy? Such that when J is moved to I, I gets the contents of J and J gets destroyed? But yet I see these examples on the net

I can see the same code working in the move constructor but how can this work in assignment? It doesn't seem to make sense :S

Should std::move be used in move constructor too? If I use std::move in the constructor it crashes my program:

Image::Image(Image&& Img) : Pixels(), Info(), /*Default everything*/
{
    this->Swap(Img);
    Img.Pixels.clear();
}

The above constructor works.. If however, I use std::move in the constructor:

Image::Image(Image&& Img) : Pixels(std::move(Img.Pixels)), Info(std::move(Img.Info)), /*move everything*/
{
    //Do I need to set Img's values to default? Or does std::move do that?
}

That will crash my program when attempting to use the Object that was moved:

Image I = Image("C:/Image.bmp");
Image J = std::move(I);
I.Draw(...); //Crashes program if using std::move. If using the swap version of the constructor, it works. I assume it's because swap version defaults everything.

Upvotes: 1

Views: 365

Answers (1)

Kerrek SB
Kerrek SB

Reputation: 476990

If you support efficient swapping and move construction, then you should only have a single assignment operator, by value:

Foo & operator=(Foo rhs) { rhs.swap(*this); }

If the user passes a const-reference, then you need to make the copy anyway. If the user passes an rvalue, then construction of the local variable rhs is cheap.

In other words, out of the three: copy/move constructor, copy/move assignment, and swap, you only need to implement two deeply (and one of those two should be the constructor).

Upvotes: 3

Related Questions