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

[Galdor]

Hi all,

I am doing a small exercise before I start implementing the rule of three (five in my case) in a larger code. However the class I am interested in has constant member variables. The reason why I declared them as const is simply that for a given object they will never change during the object's lifetime, yet the values they will hold are not known at compile-time. However, the member variables' const specifier prevents me from implementing "properly" the move-assignement operator (at least, as far as I know…). For similar reasons I can't use the copy-swap idiom for the copy constructor…

#include <memory>
#include <string>

class ArrayWrapper
{

private:

    const std::strin        _lbl; //Label for array
    std::unique_ptr<double> _arr; //Wrapped array

public:

    //DUMMY CONSTRUCTOR

    ArrayWrapper(void)
    {
        //Do nothing
    }

    // "REGULAR" CONSTRUCTOR

    ArrayWrapper
            (const std::string lbl  //in   :Array label
            )
        :_lbl (lbl )
    {
        _arr.reset( new double[10] );
    }

    // DESTRUCTOR

    //No need for explicit destructor (?)

    // COPY CONSTRUCTOR

    ArrayWrapper
            (const ArrayWrapper & other //in   :Array to copy
            )
        :_lbl (other._lbl )
    {
        std::copy(
                other._arr.get()      ,
                other._arr.get() + 10 ,
                _arr.get()            );
    }

    // MOVE CONSTRUCTOR

    ArrayWrapper
            (ArrayWrapper&& other
            )
        :_lbl ( std::move(other._lbl) ) //No compiler error but will const-declared _lbl actually be moved ?!
        ,_arr ( std::move(other._arr) )
    {
        //Do nothing
    }

    // COPY-ASSIGNMENT OPERATOR

    ArrayWrapper &
    operator=
            (ArrayWrapper other //in   :Array to assign
            )
    {
        std::swap( _lbl , other._lbl  ); //Compiler error because _lbl is declared as const
        return *this;
    }

    // MOVE-ASSIGNMENT OPERATOR

    ArrayWrapper &
    operator=
            (ArrayWrapper&& other
            )
    {
        std::swap( _lbl , other._lbl  ); //Compiler error because _lbl is declared as const
        return *this;
    }
};

I can emulate the behaviour of such functions by copying the data. However that defeats the purpose of the move-assignment operator…

    //Copy-assignment operator
    ArrayWrapper
    operator=
            (const ArrayWrapper & other //in   :Array to assign
            )
    {
        ArrayWrapper Arr(other._lbl);
        std::copy(
                other._arr.get()      ,
                other._arr.get() + 10 ,
                _arr.get()            );
        return Arr;
    }

    //Move assignment operator
    ArrayWrapper
    operator=
            (ArrayWrapper && other
            )
    {
        ArrayWrapper Arr(other._lbl);
        std::copy(
                other._arr.get()      ,
                other._arr.get() + 10 ,
                _arr.get()            );
        return Arr;
    }

To add to my confusion, here is someone who is claiming that const member variables should only be used in "const classes" (not quite sure what is meant by "const class"). He/She also say that if a class is const then it is fine to not implement the assignment operators (which appears to me as going against the rule of five…).

On a side note, could someone confirm (or refute) that since I don't have any pointers (but merely a smart pointer) I don't need to explicitly declare/implement the destructor. However, doing that would go against the rule of three (five)…

So here is a shorter list of all my questions so far :

• How should I implement the move assignment operator on a class with const member variables ?
• Is it ok to call std::move() in the move constructor in order to initialise a const member variable by swapping its value with another const member variable ? (Compiler does not complain although I was expecting it to)
• Do I need to implement a destructor whenever I have a class containing exclusively member variables that are automatically destructed ?

Looking forward for your answers !

[whiteflags]

I think a move will be fine, however, I'm inclined to agree with the poster on Stack Overflow. Constant members in classes have very limited utility. Particularly in your case, making the array label a normal mutable string and encapsulating where it changes is the same as the variable being const to the user, because they can set it in the constructor and forget about it.

class ArrayWrapper 
{
    private:
    
    std::string label_;
    std::size_t size_;    
    std::unique_pointer<double[]> arr_;  // I also think you should be using this type, since the pointer is simulating an array.

    public:
    
    ArrayWrapper(size_t size, std::string label) : label_(label), size_(size > 0 ? size : 10), arr_(new double[size])
    {
    }
    void setLabel(string label) 
    { 
        label_ = label; 
    }
    std::string getLabel() const 
    {
        return label_;
    }
};

Something like that. If the user never calls setLabel(), then it is as if label_ is const. Anywhere you can simplify your class's design choices, I really feel like you should.

There are other strategies with varying degrees of usefulness, because nevertheless, true constants do appear in classes, rarely. The clean solution is to deal with them with outside of the class proper; that is they aren't really like the rest of the members. Like having a public enum for the class and the user to use, or using a constant template argument. Or they are static members. Static members can be effectively const because all of the objects would share the same member instance.

std::move is exactly like static_cast<>ing to an r-value reference. I.e. if it compiles, it should work. The only other thing that you have to be careful for is not to do this:

string s = "my label";
s = std::move(s); // undefined: move is not required to perform self checks

[Galdor]

Particularly in your case, making the array label a normal mutable string and encapsulating where it changes is the same as the variable being const to the user, because they can set it in the constructor and forget about it.

But what about optimisation ? Does it make a difference to the compiler if the member variables are constant or not ? I always thought that I needed to be as restrictive as I possibly could… (and that's how I ended up declaring my member variables as const wherever possible…)

If the user never calls setLabel(), then it is as if label_ is const

So if I want to prevent the user from modifying label_ then I am not required to encapsulate it, correct ? On the same topic : what is considered good practice :

• to define a setters and getters for every members of the class
• or to define setters and getters for some members of the class only ?

There are other strategies with varying degrees of usefulness, because nevertheless, true constants do appear in classes, rarely

From what I understand from the examples you cite, it's never something like

class MyClass
{
    const double myVar
};

but the most similar acceptable solution would be

class MyClass
{
    static const double myVar
};

(or enum or template argument, but they look radically different). Did I understand you properly ?

std::unique_pointer<double[]> arr_; // I also think you should be using this type, since the pointer is simulating an array.

Good point !

Anywhere you can simplify your class's design choices, I really feel like you should.

That's what I'm trying to do, but unfortunately since I am not completely familiar with the language yet that's not always how my code actually ends up being…

[laserlight]

To add to my confusion, here is someone who is claiming that const member variables should only be used in "const classes" (not quite sure what is meant by "const class").

That was probably just a poor choice of words: the term "const class" or phrase "class is const" is not well defined in C++ terminology. However, from context (e.g., the next sentence) we can figure out what it means: a class for which any object of the class is logically constant.

He/She also say that if a class is const then it is fine to not implement the assignment operators (which appears to me as going against the rule of five…).

No, it does not go against the rule if you disable the assignment operators, e.g., by declaring them deleted and/or private.

[whiteflags]

But what about optimisation ? Does it make a difference to the compiler if the member variables are constant or not ?

I think you are acting prematurely. It is not clear what you want to optimize yet. If for example you are thinking about optimizing the code that displays the label, or you want to make memory reads as fast as they can be, well, these general purpose operations are basically as fast as they can be already. You should wait for a performance problem to appear before you worry about fixing it.

So if I want to prevent the user from modifying label_ then I am not required to encapsulate it, correct ? On the same topic : what is considered good practice :

You have it backwards. Since you want to make it so the label cannot change, the easiest way to do that is to make the label private, provide a get method, but not provide a set method at all. This enforces that the label does not change (whereas in previous code I didn't enforce it).

It is preferable that you hide class data behind methods such as get and set. It basically limits the surface area of the variable's impact. The point of a get method is to control where class data is used and ensures that the data is used in a read only way. You know that calling the get method is not going to cause the object to change, so the method is innocent when you are looking for and fixing bugs. You can remove the method from consideration while you are debugging.

The point of a set method is to control where and how the object changes. In your array wrapper class, it's not easy to explain get and set because basically any label will work. Still, set methods are where you can enforce business rules. Say for example that another class had an email data member; where you set that member you might want to ensure that the string had an @ in the middle, and maybe check that the domain was one of a few white-listed domains. The email example is a good one because it proves that set methods can be as complicated as you want.

In real code, the functions might not be called getFoo() and setFoo() but the idea is the same.

You understood what I had to say about class constants being static members.

[Elysia]

How should I implement the move assignment operator on a class with const member variables ?

You shouldn't because const says the member will not change ever after construction. That means you're do assignment since that, by definition, modifies the instance. If you don't want the data to change after construction, but allow copy/move assignment, then make the variables private, but non-const, then provide only getters as mentioned already.

Is it ok to call std::move() in the move constructor in order to initialise a const member variable by swapping its value with another const member variable ? (Compiler does not complain although I was expecting it to)

No! The code compiles because you're making a copy! Assignment modifies the instance! With your code, if I do

MyClass a, b;
a = b;

a will not actually equal b. Instead, the operator will return a new instance, c, which will be equal to a. This is not intuitive and goes against what we're taught about assignment.

Do I need to implement a destructor whenever I have a class containing exclusively member variables that are automatically destructed ?

No you don't. But the thing is - BECAUSE you're using RAII pattern here, there is no need for any copy constructor, move constructor copy or move assignment operators. The compiler will generate all the code automatically. The point is that if you need to do something special with your members, you probably need to implement special logic in all places.

Btw, use std::vector for your array. As it stands now, you're just asking for trouble if you change the array size. You will most likely end up with a bug.

As for getters and setters... you shouldn't make them out of habit for every member. You should strive to define what kind of data you want your class to return. If that information just means returning a member variable, then make a getter for that, then. But don't blindly make getters and setters for every variable.

[Galdor]

However, from context (e.g., the next sentence) we can figure out what it means: a class for which any object of the class is logically constant.

Yet, whether the object should be constant or not is a choice made by the class user, not by the person who wrote the class… I can't prevent the user from creating non-const objects, right ?

No, it does not go against the rule if you disable the assignment operators, e.g., by declaring them deleted and/or private.

All right !

I think you are acting prematurely. It is not clear what you want to optimize yet. If for example you are thinking about optimizing the code that displays the label, or you want to make memory reads as fast as they can be, well, these general purpose operations are basically as fast as they can be already. You should wait for a performance problem to appear before you worry about fixing it.

Okey dokey !

It is preferable that you hide class data behind methods such as get and set.

Sure, up to now I declared as many of the member variables as private and defined the least getter and setter methods possible, only using them when I could not think of any better alternative.

As for getters and setters... you shouldn't make them out of habit for every member.

All right !

BECAUSE you're using RAII pattern here, there is no need for any copy constructor, move constructor copy or move assignment operators. The compiler will generate all the code automatically. The point is that if you need to do something special with your members, you probably need to implement special logic in all places.

I thought the member variable

std::unique_ptr<double[]> _arr;

was the thing special that required me to implement that "special logic" : I want to deep copy the array's content (or move it)… You are implying it is not necessary to do so ?

Btw, use std::vector for your array. As it stands now, you're just asking for trouble if you change the array size. You will most likely end up with a bug.

In my case the array's dimensions never change after instantiation. Also it can be a fairly big array (up to 15,000,000 items) so I'm not sure std::vector is the better option. Do you think otherwise ?

No! The code compiles because you're making a copy!

I'm not sure we are talking about the same thing : that remark of mine concerned the move constructor,

ArrayWrapper
            (ArrayWrapper&& other
            )
        :_lbl ( std::move(other._lbl) ) //No compiler error but will const-declared _lbl actually be moved ?!
        ,_arr ( std::move(other._arr) )
    {
        //Do nothing
    }

not the copy- or move-assignment operators… Here std::move() is called with other._lbl as an argument… eventhough ArrayWrapper::_lbl is declared as const and hence shouldn't be moved ? (at least that what I expected but to my surprise the compiler seemed happy with it)

On the same subject :

std::move is exactly like static_cast<>ing to an r-value reference.

@whiteflags I didn't quite understand that bit…

[King Mir]

In my case the array's dimensions never change after instantiation. Also it can be a fairly big array (up to 15,000,000 items) so I'm not sure std::vector is the better option. Do you think otherwise ?

Vector is still a good option, but you should reserve() that number of elements before you build the array. There's no cost to having, but not using, the ability to add elements after initialization.

If you run out of memory, you might consider using a deque. It has the same iterator guarantees, but it will fragment your large array, allowing it to use discontinuous memory.

I'm not sure we are talking about the same thing : that remark of mine concerned the move constructor,

ArrayWrapper
            (ArrayWrapper&& other
            )
        :_lbl ( std::move(other._lbl) ) //No compiler error but will const-declared _lbl actually be moved ?!
        ,_arr ( std::move(other._arr) )
    {
        //Do nothing
    }

not the copy- or move-assignment operators… Here std::move() is called with other._lbl as an argument… eventhough ArrayWrapper::_lbl is declared as const and hence shouldn't be moved ? (at least that what I expected but to my surprise the compiler seemed happy with it)

On the same subject :



@whiteflags I didn't quite understand that bit…

That's right, no move will occur.

std::move would probably be better named "move_cast". It cast an object to a "throw-away" value, which in technical parlance is called an r-value reference. You can have a const throw-away value, so operation is fine, however, you cannot actually move from a const value, because that involves modifying it. In the language of types calling std::move here casts _lbl from "const string &" to "const string &&", which then calls the "const string &" string constructor, because that's the best match.

[Galdor]

Vector is still a good option, but you should reserve() that number of elements before you build the array. There's no cost to having, but not using, the ability to add elements after initialization.

What about the time required to access vector elements ? How does it compare with double[] arrays ? And does the CPU cache the data as efficiently with std::vector as it does with double[] arrays ? (I.e. I tend to use several neighbooring values of the array one after the other.) If std::vector has so many advantages then why do people keep on using double[] typed variables ? (Unless they don't ?)

I saw that on cppreference.com :

Vectors usually occupy more space than static arrays, because more memory is allocated to handle future growth.

In my case memory can become a scarce resource… In the sense that it happened before that my RAM was completely or almost completely full…

If you run out of memory, you might consider using a deque. It has the same iterator guarantees, but it will fragment your large array, allowing it to use discontinuous memory.

Interesting to know !
But having discontinuous memory will probably impair CPU caching, right ?

std::move would probably be better named "move_cast". It cast an object to a "throw-away" value, which in technical parlance is called an r-value reference. You can have a const throw-away value, so operation is fine, however, you cannot actually move from a const value, because that involves modifying it. In the language of types calling std::move here casts _lbl from "const string &" to "const string &&", which then calls the "const string &" string constructor, because that's the best match.

So what you are saying is that std::move() attempts to move a value and if it can't then the value is copied ? In my case, the compiler is happy because even though it can't move the value, it can still copy it ?

At least that's what I understand when reading this :

This move() gives its target the value of its argument, but is not obliged to preserve the value of its source. So, for a vector, move() could reasonably be expected to leave its argument as a zero-capacity vector to avoid having to copy all the elements. In other words, move is a potentially destructive read.

It is now up to client code to overload key functions on whether their argument is an lvalue or rvalue (e.g. copy constructor and assignment operator). When the argument is an lvalue, the argument must be copied from. When it is an rvalue, it can safely be moved from.

[King Mir]

What about the time required to access vector elements ? How does it compare with double[] arrays ? And does the CPU cache the data as efficiently with std::vector as it does with double[] arrays ? (I.e. I tend to use several neighbooring values of the array one after the other.) If std::vector has so many advantages then why do people keep on using double[] typed variables ? (Unless they don't ?)

I saw that on cppreference.com :



In my case memory can become a scarce resource… In the sense that it happened before that my RAM was completely or almost completely full…

A quality implementation of reserve() should only allocate as much memory as you ask for. However, a vector does store the size of memory allocated and the number of elements, so there is a tiny amount of memory overhead, you're right.

Interesting to know !
But having discontinuous memory will probably impair CPU caching, right ?

Not so much; it's not a linked list; it's an array/list of arrays. So your array is devided into chunks, and reading from the same chunk does not impair caching. Besides, your whole array does not fit into cache anyway. But there are some additional costs to iterating.

So what you are saying is that std::move() attempts to move a value and if it can't then the value is copied ? In my case, the compiler is happy because even though it can't move the value, it can still copy it ?

At least that's what I understand when reading this :

Yep.

[Elysia]

I thought the member variable

std::unique_ptr<double[]> _arr;

was the thing special that required me to implement that "special logic" : I want to deep copy the array's content (or move it)… You are implying it is not necessary to do so ?

Sorry, I wasn't clear. If you deep copy semantics, then you're right, you need special logic. If you want a moveable-only type (which is what unique_ptr implies), then you don't need to do anything.

In my case the array's dimensions never change after instantiation. Also it can be a fairly big array (up to 15,000,000 items) so I'm not sure std::vector is the better option. Do you think otherwise ?

It it almost always the better option. It does come with a slight overhead (it needs to store the size and a pointer to the data), but that's it. It handles deep copying, reallocation and deallocation by itself. In case you only need a specific size, you can explicitly size the vector (or reserve).

I'm not sure we are talking about the same thing : that remark of mine concerned the move constructor,

ArrayWrapper
            (ArrayWrapper&& other
            )
        :_lbl ( std::move(other._lbl) ) //No compiler error but will const-declared _lbl actually be moved ?!
        ,_arr ( std::move(other._arr) )
    {
        //Do nothing
    }

not the copy- or move-assignment operators… Here std::move() is called with other._lbl as an argument… eventhough ArrayWrapper::_lbl is declared as const and hence shouldn't be moved ? (at least that what I expected but to my surprise the compiler seemed happy with it)

Yeah, that's different. In C++, copying and moving is handled by calling an appropriate function (either constructor or assignment operator). So how does the compiler know which function to call? By doing its usual function overloading evaluation, by looking at the arguments types.
Normally, other._lbl is of type const std::string. If we remove the const keyword, it becomes std::string (hereforth called T). So you have const T or T, then the compiler looks at the constructors and sees two versions: one that accepts const T& and one that accepts T&&. There is no implicit conversion T -> T&&, so the move constructor is ruled out. There is an implicit conversion T/const T -> const T&, so the copy constructor is called.
What std::move does is change the type from T to T&&, and that's it. If you've got const T, then it changes the type to const T&&. There is no implicit conversion const T&& -> T&&, so the move constructor is, again, ruled out, leaving only const T&& -> const T& for the copy constructor, where there IS an implicit conversion. Therefore, it compiles and it calls the copy constructor instead of the move constructor. 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.

[laserlight]

What about the time required to access vector elements ? How does it compare with double[] arrays ? And does the CPU cache the data as efficiently with std::vector as it does with double[] arrays ? (I.e. I tend to use several neighbooring values of the array one after the other.)

If these matter to you, you should measure and find out for yourself as implementations can vary. That said, assuming that optimisations are enabled, NDEBUG is defined, and checked iterators are disabled, the difference is likely to be negligible in the bigger picture, especially since you appear to be doing dynamic memory allocation. std::vector may even be better if you involve class types instead of double because you are not using placement new, so you may end up constructing objects that do not need to be constructed.

If std::vector has so many advantages then why do people keep on using double[] typed variables ? (Unless they don't ?)

If the size of the array that is to be in use is fixed and known at compile time (e.g., an array of all the month names), then a fixed size array, whether built-in or std::array, could be more appropriate than std::vector. Basically, std::vector is intended to be the "default" dynamic array container.

In your case, "the array's dimensions never change after instantiation", but you say it is "up to 15,000,000 items", not exactly "15,000,000 items", so std::vector sounds reasonable to me. std::unique_ptr does cater for array delete[], and this is useful when you have no choice but to deal with a built-in dynamic array or really want to avoid unnecessarily keeping track of both capacity and size, but here you do have a choice and you only have a single array per object.

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? If so, perhaps unique_ptr and vector both do not serve your purposes, i.e., you should just use an ordinary pointer and do manual memory management for practice in implementing RAII. If you are not trying to implement a simple dynamic array, but rather you want to have a dynamic array with a label, then perhaps LabelledArray (or LabeledArray, if you prefer American English spelling) would be a better name.

I saw that on cppreference.com :

Vectors usually occupy more space than static arrays, because more memory is allocated to handle future growth.

In my case memory can become a scarce resource… In the sense that it happened before that my RAM was completely or almost completely full…

I do not know if the author of that cppreference.com article actually did the research to verify such a claim. Rather, it sounds like merely a preamble that is not necessarily true, in order to introduce the topic of how memory might be allocated internally in a std::vector. In practice, I do not find it to be true: if you are using a fixed size array, you might allocate for the largest possible size, in which case even with its unused memory allocation a std::vector alternative might occupy less space if the largest possible size is not reached. If you are using a dynamic array, then to achieve the same time complexity offered by std::vector for various operations, you would end up implementing the same kind of allocation strategies, hence there will be no difference other than what small tweaks you can customise with a special understanding of the expected growth rates.

In your case, King Mir already gave you a relevant suggestion: "you should reserve() that number of elements before you build the array". This way, re-allocation is not required, so more memory will not be allocated to handle future growth, unless you insert more elements than what was reserved.

But having discontinuous memory will probably impair CPU caching, right ?

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?

So what you are saying is that std::move() attempts to move a value and if it can't then the value is copied ? In my case, the compiler is happy because even though it can't move the value, it can still copy it ?

Not quite: what King Mir is saying is that std::move is a bit of a misnomer as it does not move anything. Whether move semantics happens or not after that depends on whether it is available.

[Elkvis]

But there are some additional costs to iterating.

Maybe if you're using iterators or range-for, but if you're using a loop like this:

std::vector<double> dVector;
for (size_t i = 0; i < dVector.size(); ++i)
{
  // do something with dVector[i]
}

There should be no additional cost. When compiling with optimizations enabled, vector::size() will likely be inlined, as will operator[], making them zero-cost.

[laserlight]

Maybe if you're using iterators or range-for, but if you're using a loop like this:
(...)
There should be no additional cost. When compiling with optimizations enabled, vector::size() will likely be inlined, as will operator[], making them zero-cost.

King Mir was referring to std::deque compared to say, std::vector. There may be a small additional cost when iterating as a consequence of having separate chunks.

[Galdor]

so there is a tiny amount of memory overhead, you're right.

If it is that small then it is not so bad… I was expecting much worse. This wikipedia article states

To avoid incurring the cost of resizing many times, dynamic arrays resize by a large amount, such as doubling in size, and use the reserved space for future expansion.

So my fear is to allocate a lot more space than I really need. The cppreference article on std::vector::reserve() states that :

void reserve( size_type new_cap );

Increase the capacity of the container to a value that's greater or equal to new_cap.

So from what I understand calling reserve() might allocate more space than I truly need, right ?

Also, I still don't get it : from what I understand, you seem to say that it is never a good idea to declare a variable as a double[] array… Instead you recommend using a container class such as std::vector or std::deque. So why does the language leave me the possibility to create a variable of type double[] if it is never the best way to do things ? Is this due to historical reasons ?