What's the correct code for the move assignment in Bjarne's example Vector?

202 views Asked by At

I am reading Chapter 5 of the 2nd edition of A Tour of C++ by Bjarne Stroustrup. He is using an example implementation of Vector to convey his ideas and gets to the move constructor and shows the code but says that the move assignment is similar but doesn't show the code.

A move constructor does not take a const argument: after all, a move constructor is supposed toremove the value from its argument. A move assignment is defined similarly.

class Vector
{
public:
    Vector(int sz);                     // constructor
    Vector(const Vector &v);            // copy constructor
    Vector &operator=(const Vector &v); // copy assignment
    Vector(Vector &&v);                 // move constructor
    Vector &operator=(Vector &&v);      // move assignment
    ~Vector();                          // destructor

private:
    double *elem;
    int sz;
};

Vector::Vector(int sz) : elem{new double[sz]}, sz{sz}
{
}

Vector::Vector(const Vector &v) : elem{new double[v.sz]}, sz{v.sz} // copy constructor
{
    for (int i = 0; i != sz; ++i)
    {
        elem[i] = v.elem[i];
    }
}

Vector &Vector::operator=(const Vector &v) // copy assignment
{
    double *p = new double[v.sz];

    for (int i = 0; i != v.sz; ++i)
    {
        p[i] = v.elem[i];
    }

    delete[] elem; // delete all elements
    elem = p;
    sz = v.sz;

    return *this;
}

Vector::~Vector()
{
    delete[] elem;
}

Vector::Vector(Vector &&v) // move constructor
    : elem{v.elem},
      sz{v.sz}
{
    v.elem = nullptr;
    v.sz = 0;
}

Is this the correct move assignment operator?

Vector &Vector::operator=(Vector &&v)
{
    elem = v.elem;
    sz = v.sz;

    v.elem = nullptr;
    v.sz = 0;

    return *this;
}

How would I be able to check that a move and not a copy was made if both constructors exist?

2

There are 2 answers

3
Ted Lyngmo On

Is this the correct move assignment operator?

No, it's not since it will make the program leak any memory already allocated by *this. Normally, you'd also want your move constructor and move assignment operator to be noexcept.

You could also check for self-assignment (if (this == &v) return *this;) to make self-assignment into a no-op. Then delete[] elem; before elem = v.elem; ...

Vector& Vector::operator=(Vector&& v) noexcept
{
    if (this == &v) return *this; // optional: makes self-assignment into a no-op

    delete[] elem;

    elem = v.elem;
    sz = v.sz;

    v.elem = nullptr;
    v.sz = 0;

    return *this;
}

... or std::swap the memory with v and let v free it when it is destroyed:

Vector& Vector::operator=(Vector&& v) noexcept
{
    std::swap(elem, v.elem);
    std::swap(sz, v.sz);

    return *this;
}
0
tbxfreeware On

If the copy-assignment operator uses a by-value parameter, then the operator can serve for both copy-assignment and move-assignment.

    Vector& Vector::operator=(Vector that) noexcept
    {
        swap(that);
        return *this;
    }

Parameter that will be initialized by making a copy of the argument used at the call site. For lvalue arguments, the compiler will choose the copy constructor when making the copy. For rvalue arguments, it will choose the move constructor. Thus, this assignment operator works efficiently in both cases.

This operator also works in the case of self-assignment. Although you could include an explicit test for self-assignment, most implementations do not bother. Self-assignment should be rare, anyway. When it does happen, the compiler will construct a copy of the existing Vector, and then swap the existing Vector with the identical copy.

The only advantage gained by testing for self-assignment in the function above would be to skip the swap. You could not prevent the compiler from making a copy. The copy will have been made prior to entry of the function.

Deletion of the original vector occurs after the swap, when excecution encounters the closing brace of operator=. That's when the destructor of parameter that is invoked.

noexcept swap function

In order for this to work, you will need to implement a noexcept swap function. It is convenient, but not necessary, to have two of them. The first is a member function that takes a single argument. The second is a free function taking two arguments, which is implemented as a hidden friend.

Coding swap as its own function, rather than simply including its implementation as part of operator=, is beneficial for several reasons. See C++ Core Guidelines C83, C84, and C85 for more information. Also see this StackOverflow answer: How does "using std::swap" enable ADL?.

class Vector
{
public:
    // ...

    void swap(Vector& that) noexcept;

    // "Hidden friend"
    friend void swap(Vector& a, Vector& b) noexcept
    {
        a.swap(b);
    }
};

void Vector::swap(Vector& that) noexcept
{
    using std::swap;
    swap(this->elem, that.elem);
    swap(this->sz, that.sz);
}