7

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

Koldar
  • 1,317
  • 15
  • 35
  • 7
    Why would you pass something by const reference if you plan on modifying it? I would suggest you make a copy of the element gained by const reference, such as how the STL containers manage their data, and store the address of this data. – Arnav Borborah Feb 14 '18 at 13:36
  • So you want that the value `data` is pointing to cannot be changed, but that the pointer `data` can be changed? – t.niese Feb 14 '18 at 13:42
  • Change `T* data;` to `T const* data;`. But I advise against holding a pointer to an object that is not owned by the important_structure. – Eljay Feb 14 '18 at 13:44

3 Answers3

3

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.

Demosthenes
  • 1,515
  • 10
  • 22
  • I agree that casting const away is indeed a bad practice. That's why I asked the question :) Maybe it's a noob question, but I though that const in function parameters meant that the reference didn't change within the function. Isn't that the reason why the overloading of operator == (or +,- and so on) have all const references? – Koldar Feb 14 '18 at 15:07
  • 1
    @Koldar Such a narrow definition is not consistent with the rest of the language, otherwise you would not be getting the compiler error in the first place (i.e. that assigning a `const int *` to an `int *` variable is forbidden, even though it does not change the pointee). Also, you are inviting UB when an unsuspecting caller gives you a reference to a `const` object and unwittingly calls `change()` later. If you need to put a documentation note that the `const int &` cannot safely refer to a `const` object, you know that your design is troubled. Also, trust your language elders. ;-) – Arne Vogel Feb 14 '18 at 15:46
  • I'm trying to change my mindset: with the sentence "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" are you saying that with such function prototype I'm forced to inject to function always constants? I come from C and I know const in C++ is pretty different from C, thus my doubts – Koldar Feb 14 '18 at 18:31
1

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;
p-a-o-l-o
  • 9,807
  • 2
  • 22
  • 35
  • ok, can you please explain how do you define the field "data" within the structure? const or not? And how do you initialize it? I feel I'm missing something very important. BTW nice for the move operations... I didn't know those – Koldar Feb 16 '18 at 07:41
  • I think you wanted to wrap an instance of `T` (`data`) inside an instance of `important_structure`, but many details are missing. Where is this instance constructed/initialized? What happens after assigning it to an `important_structure` instance? Is `important_structure` the only one who can access it? If yes, we can say the `data` resource is *owned by* `important_structure`. Is this what you want? In my example I suggest to **not** hold the instance, but have methods that accepts one and explicitly say if they're only reading it or modifying it. – p-a-o-l-o Feb 16 '18 at 07:52
  • You're right: I definitevely missed lots of details. In this case `important_structure` is initialized on the heap and so does the field `data`; I prefer `important_structure` to be the main entry access of the field `data`, hence I wish to be modifiable. In this sense we can say that `data` is *owned* by `important_structure` (but access to the field `data` is not private). I feel like `important_structure` behave like a container. Hope I replied to all your questions. – Koldar Feb 16 '18 at 08:27
  • Edited again, sorry. – p-a-o-l-o Feb 16 '18 at 15:04
0

Using const T& el and data(&el) is a really bad idea, because it implies that you could write:

new important_structure<int>{5, 3};

But to write new important_structure<int>{5, 3}; would result in data holding an address that would no longer be valid immediately after calling the constructor.

If you want that the point data can be changed, but that the value where the pointer points to cannot be changed, then you want to write it that way:

template <typename T>
struct important_structure {
    public:
    T const * data;
    int a;
    important_structure(T const * 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( T const * el, int a) : data(el), a(a) { //error line 
};


int main() {
    int i = 5;
    important_structure<int>* s = new important_structure<int>{&i, 3};
}
t.niese
  • 39,256
  • 9
  • 74
  • 101
  • Yet it can't *alter data field in some way*. Or not? – p-a-o-l-o Feb 14 '18 at 14:29
  • @p-a-o-l-o in the question there is this sentence `[...]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.[...].` With the given code the object `data` is pointing to cannot be modified, but `data` itself can be changed. In `change` you could write `data = nullptr`, but not `*data = 2` – t.niese Feb 14 '18 at 14:43
  • If the OP only wants to **assign** something else to `data` in the `change` method, yours should be the correct answer, then. – p-a-o-l-o Feb 14 '18 at 14:51
  • Ok, I think I wrote the question in a vague way, my fault. Generally, `change` method may alter data not only via assignment, but changing data fields as well. The idea is that `important_structure` is just a container of an instance of `T` type. By the sentence " it clearly says that el won't be modified by the function" I meant the constructor (indeed the constructor doesn't change anything in the first function parameter given [`el`]). However, I don't want to transfer such "constness" to the data structure as well, hence my question. – Koldar Feb 14 '18 at 14:59
  • @t.niese by using `const` in the data structure I'm saying that `important_structure` `data` field is readonly. But If `important_structure` is the only access point to the particular `T` instance, I prevent anhy changes to the `T` instance, which I don't want to. This is why I'm not inclined to add constness to the data structure itself. Maybe I'm trying to solve the issue in the wrong way... – Koldar Feb 14 '18 at 15:01
  • @Koldar the const in an parameter says that it is save to pass a data to it, without it being changed directly or anytime in the function as result of passing it there. So in this case `const` must not be in the costructor, because, the element might be changed. – t.niese Feb 14 '18 at 15:03
  • ok, I think the constructor of `important_structure` perfectly fit in the situation you've described. My point is that the field in data structure does not fit: if `data` is the only pointer to said structure, and if I want to change such `data` field, the solution of putting `const` into the data structure prevents any changes. That would lead to the removal of `const` in the costructor for sake of const correctness but.... why should I sacrifice useful information for the maintainer? – Koldar Feb 14 '18 at 15:11
  • 2
    @Koldar • I am having difficulty following your explanation. The `const` would be part of the API contract. If you don't want it to be `const` (for whatever reason), don't put `const` in the API. If the API lies, then that would be terribly misleading information to the lament of the maintainer. – Eljay Feb 14 '18 at 15:43
  • But it's indeed true that the constructor of `important_structure` doesn't alter in any way `el`. Is there no way to tell this information to future developers in the constructor prototype? – Koldar Feb 14 '18 at 18:44