Stealing inside the move constructor

Question

During the implementation of the move constructor of a toy class, I noticed a pattern:

array2D(array2D&& that)
{
    data_ = that.data_;
    that.data_ = 0;

    height_ = that.height_;
    that.height_ = 0;

    width_ = that.width_;
    that.width_ = 0;

    size_ = that.size_;
    that.size_ = 0;
}

The pattern obviously being:

    member = that.member;
    that.member = 0;

So I wrote a preprocessor macro to make stealing less verbose and error-prone:

#define STEAL(member) member = that.member; that.member = 0;

Now the implementation looks as following:

array2D(array2D&& that)
{
    STEAL(data_);
    STEAL(height_);
    STEAL(width_);
    STEAL(size_);
}

Are there any downsides to this? Is there a cleaner solution that does not require the preprocessor?

Answer 1

Here is the recommended pattern:

array2D(array2D&& that)
    : data_(std::move(that.data_)),
      height_(std::move(that.height_)),
      width_(std::move(that.width_)),
      size_(std::move(that.size_))
{
    that.data_ = 0;
    that.height_ = 0;
    that.width_ = 0;
    that.size_ = 0;
}

Naturally if the data members are scalar types, the std::move isn't needed. But if you're copying this pattern around, it is helpful to include the move anyway so that when the member data aren't scalar, the std::move doesn't get forgotten.

Also if the member data have actual move constructors, then you can simply omit the body:

array2D(array2D&& that)
    : data_(std::move(that.data_)),
      height_(std::move(that.height_)),
      width_(std::move(that.width_)),
      size_(std::move(that.size_))
{
}

And if you want to generalize to types that don't have move constructors, but do have a resource-less default constructed state, you can:

array2D(array2D&& that)
    : data_(std::move(that.data_)),
      height_(std::move(that.height_)),
      width_(std::move(that.width_)),
      size_(std::move(that.size_))
{
    that.data_ = Data();
    that.height_ = Height();
    that.width_ = Width();
    that.size_ = Size();
}

I recommend ordering these statements in the same order they are declared as data members in the array2D class definition. And I find nothing wrong with the repetition of the initializer list in the body. It is a necessary and second step. There is no need to sweep it under the rug.

Answer 2

How about using template:

template<typename T> inline
void MOVE(T &dst, T &src)
{
  dst = src;
  src = 0;
}

Usage:

MOVE(data_, that.data_);

@Fred, from your comment, if you want to avoid mentioning data member twice, then:

#define STEAL(X) MOVE(X, that.X)

Usage:

STEAL(data_);

Answer 3

Initialize your own members to default, and then swap.

array2D(array2D&& that)
{
    data_ = 0;    
    height_ = 0;    
    width_ = 0;    
    size_ = 0;

    this->swap(that);
}

Even cleaner (if your compiler supports it)

array2D(array2D&& that)
: array2D() {
    this->swap(that);
}