Thread: Move-assignment operator on class with const-declared member variables

(I just noticed several posts appeared while I was writing mine, I will read them now)

Originally Posted by laserlight

That said, I notice that your class is named ArrayWrapper. I wonder if you are trying to practice implementing a simple dynamic array from scratch as a wrapper over a built-in dynamic array?

No, the code I showed in my original post is dummy source code. In reality the ArrayWrapper class wraps a double[] (or std::vector or similar) and enhances it with a whole bunch of methods and variables. The ArrayWrapper class represents a grid discretisation of a 2D field function. For instance it stores temperature measurements taken at regular intervals on a flat rectangular surface. The array label is one of the information stored in the class, but it is far from being the only one : there is also the physical size of the domain, coordinates of origin, number of ghost nodes, etc. And there is a whole bunch of methods : load, save, add, substract, compute average, compute Euclidean norm, initialise using function of cartesian coordinates, etc. Eventually the class's actual name is "DblArray" as in "Array of double" (even though it should be better called "DblField" or something)

Originally Posted by laserlight

King Mir qualified the suggestion with "If you run out of memory". So, I find your question rather inexplicable: you would prefer to bail out of whatever task the program is supposed to do with the memory because contiguous memory of the desired amount is not available rather than continue with a possible performance penalty because the memory is allocated in separate chunks?

Unless std::deque allocates discontinuous chunks of memory even when there would be enough space for a contiguous allocation. However, judging by your reaction this is not case.

Originally Posted by Elysia

Yeah, that's different. [...] Normally, other._lbl is of type const std::string. If we remove the const keyword, it becomes std::string (hereforth called T). [...] If you remove the const, std::move changes T to T&&, which matches the T&& constructor better than the const T& constructor, so it calls the move constructor.

Very nice explanation ! It is all clear for me now !

Here is the new (dummy) code :

Code:

#include <string>
#include <vector>
 
class ArrayWrapper
{
private:
    std::string         _lbl; //Label for array
    std::vector<double> _arr; //Wrapped array
public:
    ArrayWrapper(void)=default;
    ArrayWrapper(const std::string lbl) :_lbl(lbl),_arr(30) {}
    ~ArrayWrapper(void)=default;
    ArrayWrapper (const ArrayWrapper & other) :_lbl (other._lbl), _arr (other._arr) {}

    // MOVE CONSTRUCTOR
 
    ArrayWrapper
            (ArrayWrapper&& other
            )
        :_lbl ( std::move(other._lbl) )
        ,_arr ( std::move(other._arr) )
    {
        //Do nothing
    }
 
    // COPY-ASSIGNMENT OPERATOR
 
    ArrayWrapper &
    operator=
            (ArrayWrapper other
            )
    {
        std::swap( _lbl , other._lbl );
        std::swap( _arr , other._arr );
        return *this;
    }
 
    // MOVE-ASSIGNMENT OPERATOR
 
    ArrayWrapper &
    operator=
            (ArrayWrapper&& other
            )
    {
        std::swap( _lbl , other._lbl  );
        std::swap( _arr , other._arr  );
        return *this;
    }
};

int main(void) {
    return 0;
}

Does that look better ?

Originally Posted by Elysia

You don't need any assignment operators or copy/move constructors because the compiler will generate the correct ones for you.
Also remove the (void) in the parameter lists. They're not needed.

Is that better : (I'm sorry but I really love specifying (void) in the parameter lists)

Code:

#include <iostream>
#include <string>
#include <vector>
  
class ArrayWrapper
{
private:
    std::string         _lbl; //Label for array
    std::vector<double> _arr; //Wrapped array
public:
    ~ArrayWrapper(void)=default;                               //Destructor
    ArrayWrapper(ArrayWrapper&& other)=default;                //Move constructor
    ArrayWrapper & operator=(ArrayWrapper&& other)=default;    //Move assignment
  
    // CONSTRUCTOR

    ArrayWrapper
            (const std::string lbl
            ,std::initializer_list<double> iniLs = {}
            )
        :_lbl(lbl  )
        ,_arr(iniLs)
    {
        //Do nothing
    }

    // COPY CONSTRUCTOR

    ArrayWrapper
            (const ArrayWrapper & other
            ,const std::string    lbl=""
            )
        :_lbl(lbl=="" ? other._lbl : lbl)
    {
        _arr.clear();
        std::copy(
                other._arr.begin()       ,
                other._arr.end()         ,
                std::back_inserter(_arr) );
    }

    // COPY ASSIGNMENT

    ArrayWrapper &
    operator=
            (ArrayWrapper other
            )
    {
        std::swap(_arr, other._arr);
        return *this;
    }
    
    void
    dispContent(void)
    {
        std::cout << "Array \"" << _lbl << "\" : ";
        for(const auto elem : _arr) {
            std::cout << elem << ", ";
        }
        std::cout << std::endl;
    }
};
 
int main(void) {
    ArrayWrapper ArrA("Alpha", {1.0, 2.0, 3.0});                //
    ArrayWrapper ArrB("Beta" , {4.0, 5.0, 6.0});                //Constructor
    ArrayWrapper ArrC(ArrA, "Gamma");                           //Copy constructor
    ArrayWrapper ArrD("Delta");                                 //Constructor
    ArrayWrapper ArrE(ArrayWrapper("Epsilon", {11.0, 12.0} ));  //Move constructor
    //
    ArrA.dispContent();
    ArrB.dispContent();
    ArrC.dispContent();
    ArrD.dispContent();
    //
    ArrayWrapper ArrF = ArrayWrapper("Delta", {7.0, 8.0, 9.0}); //Move assignment
    ArrF.dispContent();
    //
    ArrF = ArrB; //Copy assignment
    ArrF.dispContent();
    //
    return 0;
}

Originally Posted by King Mir

Several reasons:
1) std::vector is a library feature that is implemented using dynamically allocated arrays (like the result of new double[N]); it is not a magic macro.
2) The language gives you the tools to write a vector like container that may be better than std:vector for your particular project.
3) historic reasons; old code still works
4) It's not a design goal of the language to prevent people from intentionally violating what are though of as best practices
5) C compatibility

Makes sense !

I think you missed the point of Elysia's post. Your class is made up of things that C++ already knows how to copy or move. In effect, vector and string already have functions that will be called for you to do that... so you don't have to write a copy constructor or assignment operator or move constructor or move assignment. (All you're doing is writing down stuff that would happen anyway.)

Sorry for the late answer but I thought this topic was closed ; Apparently it's not…

Originally Posted by whiteflags

I think you missed the point of Elysia's post. Your class is made up of things that C++ already knows how to copy or move. In effect, vector and string already have functions that will be called for you to do that... so you don't have to write a copy constructor or assignment operator or move constructor or move assignment. (All you're doing is writing down stuff that would happen anyway.)

Is this better ?

IMHO I think I still need to define a copy constructor because I want to allow the user to set a label for the newly created array that can be different from the label of the copied array.

Code:

#include <iostream>
#include <string>
#include <vector>
   
class ArrayWrapper
{
private:
    std::string         _lbl; //Label for array
    std::vector<double> _arr; //Wrapped array
public:
    ~ArrayWrapper(void)=default;                               //Destructor
    ArrayWrapper(ArrayWrapper&& other)=default;                //Move constructor
    ArrayWrapper & operator=(ArrayWrapper&& other)=default;    //Move assignment
    ArrayWrapper & operator=(ArrayWrapper & other)=default;    //Copy assignment
   
    // CONSTRUCTOR
 
    ArrayWrapper
            (const std::string lbl
            ,std::initializer_list<double> iniLs = {}
            )
        :_lbl(lbl  )
        ,_arr(iniLs)
    {
        //Do nothing
    }
 
    // COPY CONSTRUCTOR
 
    ArrayWrapper
            (const ArrayWrapper & other
            ,const std::string    lbl=""
            )
        :_lbl(lbl=="" ? other._lbl : lbl)
    {
        _arr.clear();
        std::copy(
                other._arr.begin()       ,
                other._arr.end()         ,
                std::back_inserter(_arr) );
    }
     
    void
    dispContent(void)
    {
        std::cout << "Array \"" << _lbl << "\" : ";
        for(const auto elem : _arr) {
            std::cout << elem << ", ";
        }
        std::cout << std::endl;
    }
};
  
int main(void) {
    ArrayWrapper ArrA("Alpha", {1.0, 2.0, 3.0});                //
    ArrayWrapper ArrB("Beta" , {4.0, 5.0, 6.0});                //Constructor
    ArrayWrapper ArrC(ArrA, "Gamma");                           //Copy constructor
    ArrayWrapper ArrD("Delta");                                 //Constructor
    ArrayWrapper ArrE(ArrayWrapper("Epsilon", {11.0, 12.0} ));  //Move constructor
    //
    ArrA.dispContent();
    ArrB.dispContent();
    ArrC.dispContent();
    ArrD.dispContent();
    //
    ArrayWrapper ArrF = ArrayWrapper("Delta", {7.0, 8.0, 9.0}); //Move assignment
    ArrF.dispContent();
    //
    ArrF = ArrB; //Copy assignment
    ArrF.dispContent();
    //
    return 0;
}

in fact that is the only change I would really make; delegate constructors:

Code:

ArrayWrapper(ArrayWrapper const &) = default;

ArrayWrapper(ArrayWrapper const & other, string const & label): ArrayWrapper(other)
{
   _lbl = label;
}

The reason it's like this is it forces them to pass something other then the ``other" label if that's what they want. If they don't pass anything, then this isn't called, but the default copy constructor is called instead.

Originally Posted by whiteflags

in fact that is the only change I would really make; delegate constructors:

Code:

     ArrayWrapper(ArrayWrapper const &) = default;

     ArrayWrapper(ArrayWrapper const & other, string const & label): ArrayWrapper(other)
     {
        _lbl = label;
     }

I did try this but unfortunately it did not work… Clang++ returns following error :

Code:

main.cpp:49:10: error: call to constructor of 'ArrayWrapper' is ambiguous

And g++ returns :

Code:

main.cpp:36:28: error: call of overloaded ‘ArrayWrapper(const ArrayWrapper&)’ is ambiguous
             :ArrayWrapper(other)

Which I can understand… So I am not sure that there can be a better way than what I showed in my previous post

Code:

    // COPY CONSTRUCTOR
 
    ArrayWrapper
            (const ArrayWrapper & other
            ,const std::string    lbl=""
            )
        :_lbl(lbl=="" ? other._lbl : lbl)
    {
        _arr.clear();
        std::copy(
                other._arr.begin()       ,
                other._arr.end()         ,
                std::back_inserter(_arr) );
    }

Or perhaps there is ?

Originally Posted by Elkvis

That's not a copy constructor then. A copy constructor is defined as a constructor that accepts a const reference to an object of its own type, as its only parameter.

No, a copy constructor can have default arguments, judging from cppreference

A copy constructor of class T is a non-template constructor whose first parameter is T&, const T&, volatile T&, or const volatile T&, and either there are no other parameters, or the rest of the parameters all have default values.

and by the standard

A non-template constructor for class X is a copy constructor if its first parameter is of type X&, const X&,
volatile X& or const volatile X&, and either there are no other parameters or else all other parameters
have default arguments (8.3.6). [ Example: X::X(const X&) and X::X(X&,int=1) are copy constructors.