0

In all standard containers like std::map or std::vector there is a move constructor and a move assignment to avoid copying. I want to build my own Wector class with the same functionalities. My class declaration looks as follows:

class Wector{

public:
    ~Wector();
    Wector();
    Wector(std::size_t s);
    Wector(std::size_t s, const double nr);
    Wector(const std::initializer_list<double> il);

    //copy constructor
    Wector(const Wector & w);
    //move constructor
    Wector(Wector&& w);

    std::size_t size() const;

    double* begin(); // iterator begin
    double* end(); // iterator end
    const double* begin() const; // const iterator begin
    const double* end() const; // const iterator

    const double& operator[](std::size_t i)const;
    double& operator[](std::size_t i);

    Wector& operator=(const Wector& w);
    
    //move assignment
    Wector& operator=(Wector&& w);

private: 
    std::size_t wsize;
    void swap(Wector& w);
    double *element; // double *element = new double[s]
};

To implement the move-assignment and constructor I need a customer swap.

//move constructor
Wector::Wector(Wector&& w)
    : Wector() 
{
    swap(w);
}


//move assignment
Wector& Wector::operator=(Wector&& w){
    swap(w);
    return *this;
}

But I have no idea how to implement the swap function without having direct access to the data element and without copying with help of the iterators.

void Wector::swap(Wector& v){
    std::swap(wsize, v.size());
    double * temp = new double[v.size()];
    std::copy(w.begin(), w.end(), temp);
    std::swap(element, temp);
    delete [] temp; //edited

}

Does anybody know how it is implemented in the case of std::vector?

Suslik
  • 929
  • 8
  • 28
  • 4
    Why not just `std::swap(element, v.element);`? – Alan Birtles Oct 05 '22 at 07:50
  • 1
    Just swap each element by id. And related, your first line, `std::swap(wsize, v.size())` won't compile, and `new` appearing in a swap-mechanic is the first sign things are going horribly wrong (and they are, as you not only don't gain the advantage of move semantics, you leak memory in the process). You may be interested in reading up on the [copy/swap idiom](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) where you get much of what you appear to seek as part of the model. – WhozCraig Oct 05 '22 at 07:54
  • Thank you for your answer - it works. But I still do not understand correctly, how it is possible to have access to the "old" pointer `v.element` with having `element` as a private field. – Suslik Oct 05 '22 at 08:04
  • 1
    Your `swap` is (unfortunately) a *member function*. They have access to *all* the enclosing class member vars, as well as any that are in base classes with proper permissibility granted. And before you ask why I said 'unfortunately', I direct you [here](https://stackoverflow.com/questions/5695548/public-friend-swap-member-function) – WhozCraig Oct 05 '22 at 08:05

3 Answers3

2

You can just swap the pointers themselves (and, of course, the sizes). It doesn't matter which instance allocated the storage and which deletes it, it'll only belong to one instance at a time.

lorro
  • 10,687
  • 23
  • 36
0

The swap you want will be something like:

void Wector::swap(Wector& v){
    std::swap(wsize, v.wsize);
    std::swap(element, v.element);
}
cptFracassa
  • 893
  • 4
  • 14
-1

Your swap is not a move but a copy. You can just move each member:

//move assignment
Wector& Wector::operator=(Wector&& w){
    // instead of "delete element" we use the correct Destructor for delete the old data. 
    // But for this the swap method should be optimized by exchange just the pointer and the size! 
    Vector().swap(this); // old vector is swapped in temporary object and then cleaned up in destuctor 
    // members of w are accessible becasuse operator= is a class member function.
    // NOTE: surround with std::move is superfluous here, I do it for clarity of expression/intention. 
    // Unfortunately the C++ standard does not reset the values to default, so we do it afterwards.
    this->wsize = std::move( w.wsize );
    this->element = std::move( w.element );
    // set values of the moved object to 0 
    // (we must avoid double deleting element!)
    w.wsize = 0;
    w.element = nullptr;
    return *this;
};

Same you can do for the Move Constructor.

EDIT fixed cleanup the old data.

EDIT well, this brings me to the right solution:

//move assignment
Wector& Wector::operator=(Wector&& w){
    // *** using 2 swaps for make the move! ***
    // But for this the swap method should be optimized by exchange just the pointer and the size! 

    // first exchange the values
    this->swap( w );
    // if swap did not throw (in this example the optimized swap can be made noexcept)
    // this now has the new values and w the old.
    // Now delete the old values by using a new temporary which will use the destructor for cleanup.
    Vector().swap(w); 
    // w is empty now and old data deleted.
    
    return *this;
};
  • You may be interested by [`std::exchange`](https://en.cppreference.com/w/cpp/utility/exchange) – Fareanor Oct 05 '22 at 08:16
  • The data that was allocated to this->element before calling posted operator= leaks. – Öö Tiib Oct 05 '22 at 08:17
  • Oh, thanks, Öö Tiib, I better should have use the Move Constructor as an example. I will correct it, I was in a hurry as I answered, my failure. sorry for that! – TeaAge Solutions Oct 05 '22 at 09:50