is using const_cast regularly as a design tool acceptable?

Question

I reviewed the kind of code below, and while I have a personal answer to the question (*), I'd like to have comments from C++/design experts.

For some reason, Data is an object with a non-modifiable identifier, and a modifiable value:

class Data
{
   const Id    m_id ;        // <== note that m_id is a const member variable
   Value       m_value ;

   Data(const Id & id, const Value & value) ;

   Data(const Data & data) ;
   Data & operator = (const Data & data) ;

   // etc.
} ;

The design choice became a language choice as the identifier was declared const at class level (**), to avoid its (accidental) modification even from inside the class member functions...

... But as you can see, there is a copy-assignment operator, which is implemented as:

Data & Data::operator = (const Data & that)
{
   if(this != &that)
   {
      const_cast<Id &>(this->m_id) = that.m_id ;
      this->m_value                = that->m_value ;
   }

   return *this ;
}

The fact the copy assignment operator is not const-qualified makes this code safe (the user can only legally call this method on a non-const object without provoking undefined behavior).

But is using const_cast to modify an otherwise const member variable a good class design choice in C++?

I want to stress the following:

• Data is clearly a value type (it has an operator = member function)
• in this pattern, a few other functions could legitimately need const_cast, too (move constructors/assignment and/or swap functions, for example), but not a lot.

Note that this could have been a code review question, but this is not a "day-to-day" code. This is a general C++ type design question, with the need to balance the needs/power of a language and the code interpretation of patterns/idioms.

Note, too, that mutable (as in C++98) is not a solution to the problem, as the aim is to make a member variable as unmodifiable as it can be. Of course, mutable (as in C++11 post-"you don't know const and mutable" by Herb Sutter) is even less a solution.

(*) I can privately forward my answer to that question to anyone who asks.

(**) Another solution would have been to make the object non-const, and making it const at the interface level (i.e. not providing functions that can change it)

Answer 1

Quote from cppreference:

Even though const_cast may remove constness or volatility from any pointer or reference, using the resulting pointer or reference to write to an object that was declared const or to access an object that was declared volatile invokes undefined behavior.

That means that your copy assignment is not safe, but plain incorrect. If you declare something const, you cannot ever change it safely. This has nothing to do with design.

The only valid uses of const_cast is removing constness from a const reference or pointer to a non-const object (or to a const object and then not modifying it, but then you can just not const_cast instead).

Answer 2

I will utilize it when I implement the lone accessor to a private member where it returns a const reference, i.e. the clients of this class only see a const reference.

However, when a derived class needs to "modify" the private member, I could implement a non-const protected accessor, but I would rather limit the derived class's accessor calls to the const-reference, where most of the time it only needs the const reference anyway.

So then, in the few situations where I do need to "tweak" it in the derived class, the const_cast<> sticks out like a sore thumb, but this is by choice. I like that it sticks out. I can easily search for it (who is const_cast<>-ing this class?).

The alternative - providing a protected non-const accessor, may be more "correct" grammatically, but I'd rather make the non-const access be obtrusive, not "ordinary".

Answer 3

Normally a class should have full control and knowledge about its own members. Your requirement to protect a member from missuse in it's own class is against some basic object oriented design principles.

Of cause one can declare a private variable as constant, if it's really constant. But in your case you only want to protect it from some methods. In this case keep it non-constant, or split the class. You can use something like the Private class data pattern to gain finer control of the accessablity of variables.

Answer 4

Even if I'm not a design expert, let alone a C++ expert, I think this is a case of "design gotcha" (I allow myself to say it because certain of the fact that the trap is done artfully).

In my opinion argument starts from the wrong assumption that "Data is clearly a value type" and then turning into some "constness" issue.

The "value" of a Data object is the combination of ID and Value, and the "keyability" of Value by an Id determines a uniqueness for (Id, Value) pair. In other words, it is the Id-> Value correspondence to characterize itself as constant, but in the sense of univocal.

Furthermore, if a Data object is born as a Id-> Value correspondence that for some reason is no longer valid (in the sense that must be modified) then Data itself has concluded its life cycle, so it does NOT change. From this point of view we come to the characterization of immutable objects.

I would implement it with something like the following code, where the KeyedValue class template encapsulates the requirements outlined above by drawing from a pool of objects returned by reference:

template <class K, class V>
class KeyedValue {
public:
    typedef K key_type;
    typedef V value_type;

    const K& key() const { return _key; }
    const V& value() const { return _value; }

    operator K() const { return _key; }
    //bool operator == (const Keyable& other) { return _key == other.key(); }
    /**************************/
    /* _value doesn't take part to hash calculation */
    /* with this design choice we have unique KeyedValue(s) */
    struct hash {
        size_t operator()(const KeyedValue& d) const noexcept {
            return std::hash<K>()(d.key());
        }
    };
    /**************************/
    static KeyedValue getValue(const K& key, const V& val));

private:
    KeyedValue& operator = (const KeyedValue&); // Don't implement
    K _key;
    V _value;
protected:
    KeyedValue(const K& key_val, const V& val):  _key(key_val), _value(val) {}
    static std::unordered_set<KeyedValue<K, V>, typename KeyedValue<K, V>::hash> value_pool;
    };

template <class K, class V>
std::unordered_set<KeyedValue<K, V>, typename KeyedValue<K, V>::hash> 
KeyedValue<K, V>::value_pool;

template <class K, class V>
KeyedValue<K, V> KeyedValue<K, V>::getValue(const K& key, const V& val) {
    KeyedValue to_find(key, val);
    auto got = value_pool.find (to_find);
    if (got == value_pool.end()) {
        value_pool.insert(to_find);
        return to_find;
    }
    else
        return *got;
}

typedef size_t Id;
typedef int Value;
typedef KeyedValue<Id, Value> Data;