Const correctness in struct initialization

Question

I'm playing with C++ and const-correctness right now. Assume you have the following structure

template <typename T>
struct important_structure {
    public:
    T* data;
    int a;
    important_structure(const T& el, int a);
    void change();
};

template <typename T>
void important_structure<T>::change() {
    //alter data field in some way
}

template <typename T>
important_structure <T>::important_structure(const T& el, int a) : data(&el), a(a) //error line {
};


int main() {
    important_structure<int>* s = new important_structure<int>{5, 3};
}

When compiling with std=c++11, the compiler returns the following error:

invalid conversion from ‘const int*’ to ‘int*’

Now, I know it's unsafe to cast a const int* to int*. The problem is that I have a data structure and I don't want to put the field data as a constant.

However, I don't want to remove the const qualifier in the constructor since, I think, it's informative for future developers: it clearly says that el won't be modified by the function. Still the field data may be modified by some other function in important_structure.

My question is: How can I deal with fields which are initialized in the costructor and altered in some other function? Most of const correctness deals with simple answers, but no question (I think) deals with scenarios where a const parameter is passed to a data structure and then such data structure is altered by someone else.

Thanks for any kind reply

Answer 1

passing el as a const reference doesn't just mean the function will not change el during the run of the function, it means because of this function call, el won't be changed at all. And by putting the address of el into non-const data, you violate that promise.

So, the clean solution, if you indeed want to change data, is removing the const. since it is not informative to future developers, but misleading. Casting away the const would be very bad here.

Answer 2

Let's use a simple class as T type of important_struct:

class Data
{
public:
    Data() : something(0){}
    Data(int i) : something(i){}
    Data(const Data & d) : something(d.something){}

    //non-const method: something can be modified
    void changeSomething(int s){ something += s; }

    //const method: something is read-only
    int readSomething() const { return something; } 

private:
    int something;
};

This class has a very simple, yet well encapsulated, status, i.e. the int something field, which is accessed through methods in a very controlled way.

Let (a simplified version of) important_structure hold an instance of Data as a private field:

template <typename T>
struct important_structure
{
public:
    important_structure(T * el);
    void change();
    int read() const;
private:
    T* data;
};

We can assign a Data instance to an important_structure instance this way:

important_structure<Data> s(new Data());

The instance is assigned in construction:

template <typename T>
important_structure <T>::important_structure(T * el) : data(el) {}

Now the great question: do important_structure take ownership of the Data instances it holds? The answer must be made clear in documentation.

If it is yes, important_structure must take care of memory cleanup, e.g. a destructor like this one is required:

template<typename T>
important_structure<T>::~important_structure()
{
    delete data;
}

Notice that, in this case:

  Data * p = new Data()

  // ...

  important_structure<Data> s(p);

  //p is left around ...

another pointer to the Data istance is left around. What if someone mistakenly call delete on it? Or, even worse:

  Data d;

  // ...

  important_structure<Data> s(&p); //ouch

A much better design would let important_structure own its own Data instance :

template <typename T>
struct important_structure
{
public:
    important_structure();
    void change();
    // etc ...
private:
    T data; //the instance
};

but this is maybe simplistic or just unwanted.

One could let important_structure copy the instance it will own:

template<typename T>
important_structure<T>::important_structure(const T &el)
{
    data = el;
}

the latter being the constructor provided in the question: the object passed won't be touched, but copied. Obviously, there are two identical Data objects around, now. Again, the result could not be what we needed in the first place.

There is a third way, in the middle: the object is instantiated outside the owner, and moved to it, using move semantics.

As an example, let's give Data a move assignment operator:

Data & operator=(Data && d)
{
    this->something = d.something;
    d.something = 0;
    return *this;
}

and let important_structure provide a constructor which accepts an rvalue reference of T:

important_structure(T && el)
{
    data = std::move(el);
}

One can still pass a Data instance using a temporary as the required rvalue:

important_structure<Data> s(Data(42));

or an existing one, providing the required reference from an lvalue, thanks to std::move:

Data d(42);

// ...

important_structure<Data> x(std::move(d));
std::cout << "X: " << x.read() << std::endl;
std::cout << "D: " << d.readSomething() << std::endl;

In this second example, the copy held by important_structure is considered the good one while the other is left in a valid but unspecified state, just to follow the standard library habits.

This pattern is, IMHO, more clearly stated right in code, expecially if considered that this code will not compile:

Data d(42);
important_structure<Data> x (d);

Whoever wants an instance of important_structure must provide a temporary Data instance or explicitly move an existing one with std::move.

Now, let the important_structure class be a container, as you asked in comment, so that data is somehow accessible from outside. Let's give a method like this to the important_structure class:

const T & owneddata() { return data; }

Now, we can use data const methods like this:

important_structure<Data> s(Data(42));

std::cout << s.owneddata().readSomething() << std::endl;

but calls to `Data' non-const methods will not compile:

s.owneddata().changeSomething(1000); //not compiling ...

If in need of it (hope not), expose a non-const reference:

T & writablereference() { return data; }

Now the data field is at full disposal:

s.writablereference().changeSomething(1000); //non-const method called
std::cout << s.owneddata().readSomething() << std::endl;