Both const and non-const version of the same function - an anti-pattern?

Question

Recently I've inspected a huge amount of legacy C++ code and found something I've never seen before in production C++ code:

class Foo
{
public:
    void Bar()
    {
        std::cout << "Hello from Bar()!" << std::endl;
    }

    void Bar() const 
    {
        const_cast<Foo*>(this)->Bar(); 
    }
};

Is this a huge anti-pattern? I mean, the function is either const or non-const, what's the point of providing two versions? Is this some kind of 'const-correctness cheat', that allows to invoke const functions is situations like this:

void InvokeBar(const Foo& foo)
{
    // oh boy! I really need to invoke a non-const function on a const reference!
    foo.Bar();
}

`const_cast` **is** the const-correctness cheat. It tells the compiler to let you do something it can't prove is correct. — Brian Bi, Oct 23 '15 at 18:35
There *are* legitimate uses of having two member functions with the same name with one const and the other not, such as the `begin` and `end` iterator functions, which return non-const iterators on non-const objects, and const iterators on const objects, but if it's casting from const to do something, it smells like fish. — jaggedSpire, Oct 23 '15 at 18:38
The classic example is the overload of `operator [ ]` http://en.cppreference.com/w/cpp/container/vector/operator_at — PaulMcKenzie, Oct 23 '15 at 18:44

The Paramagnetic Croissant · Accepted Answer · 2015-10-23T18:43:26.470

9

No, not always.

There are legitimate uses for this pattern. For example, suppose you are writing a collection, and the code for retrieving an element is fairly complex (e.g. a hash table). You don't want to duplicate all the code, but you also want your collection to be able to be used as both const and non-const.

So, you might do something like this:

struct HashTable {
    ...

    const Value &get(Key key) const {
        ... complex code for retrieving the key
    }

    Value &get(Key key) {
        return const_cast<Value &>(
            static_cast<const HashTable *>(this)->get(key)
        );
    }
};

Here, the const_cast<> is not really a lie. Since your function is non-const, you know that it can be called only if the object pointed to by this is also non-const. Hence, casting the constness away is valid.

(of course, similarly to this situation, you can call a non-const method by casting away the const-ness of a const instance, but at that point its the user of your class who has already introduced undefined behavior, so you are covered as long as your class is being used correctly.)

edited Oct 23 '15 at 18:43

answered Oct 23 '15 at 18:39

The Paramagnetic Croissant

9,223
3
25
38

But that most likely should be implemented with the way tou are pointing when you have `const` version that does things and _non-const_, that calls _const_. – Lol4t0 Oct 23 '15 at 18:42
@Lol4t0 Yes, probably. – The Paramagnetic Croissant Oct 23 '15 at 18:43
this example is certainly an anti-pattern. The 'complex code' should be in a const method. – Richard Hodges Oct 23 '15 at 18:44
@RichardHodges is it not in my example? – The Paramagnetic Croissant Oct 23 '15 at 18:45
...Otherwise you can encourage undefined behaviour if non-const function of _true const_ object is called and changes the object – Lol4t0 Oct 23 '15 at 18:46
@TheParamagneticCroissant the method of finding the location of the element is logically distinct from the method of returning it to the caller. the proposed example lays a landmine for a maintainer IMHO. – Richard Hodges Oct 23 '15 at 18:51
@RichardHodges This example is simplified, I didn't want to write a full, actual hash table implementation in a Stack Overflow answer, excuse me for that. But **somewhere** you will **have to** get away with the `const` qualifier. Even if you separate the "find the bucket for the key" code in a distinct method (which I obviously do – I **have** written a hash table like this before!), you have to decide which type it returns. Const or non-const? If the former, you will need to cast it away in the non-const overload of `get()`. If the latter, its logic itself will need to `const_cast<>`. – The Paramagnetic Croissant Oct 23 '15 at 18:54
@RichardHodges The point is, `const_cast` can be used as a technique for refactoring. The actual implementation details are irrelevant. – The Paramagnetic Croissant Oct 23 '15 at 19:00
converting the 'find the bucket' method to a template method removes the need for const_cast plus adds compile-time safety. there is no need for const_cast here. – Richard Hodges Oct 23 '15 at 19:16
@RichardHodges There's no "unsafety" in this usage (see last two paragraphs), so that's not really a concern. Templatizing upon constness seems a worse thing to do for me. – The Paramagnetic Croissant Oct 23 '15 at 19:45
I think before we can reason on what's worse we have to agree on aims. My aim would favour preventing a maintainer from making a mistake. Const cast to me is inviting a mistake because it's giving up compiler protection, in this case for zero (by my standards) gain. However, here is not the place to argue tit for tat. There are many ways to skin a cat. – Richard Hodges Oct 23 '15 at 19:54

Both const and non-const version of the same function - an anti-pattern?

1 Answers1