Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sometimes I run across the problem of having two member function overloads that only differ in the constness of this (and the return type, but that is less important):

int& operator[](int);
const int& operator[](int)const;

Most of the time these are one-liners, but even then, I hate the code duplication. That's why I like to use (if possible) the const version to implement the non const version via const_cast:

const int& MyClass::operator[](int index)const {
    return const_cast<int&>(const_cast<MyClass&>(*this)[index]);
}

The first const_cast is required (at least I think so), but I wonder if the second one can be circumvented and if so, if the code is more readable then.

How do you solve this problem?

share|improve this question
 
There are several good answers on StackOverflow: see How do I remove code duplication between similar const and non-const member functions? –  ChrisW yesterday
add comment

3 Answers

up vote 4 down vote accepted

As ChrisW commented, this has been covered in depth at Stack Overflow. You're very close, but you should actually have the non-const version cast away the const-ness of the const version rather than how you're doing it.

If you're calling a non-const method, you are operating on a non-const object. This means that the const cast is technically valid since you are assured that the underlying object is non-const anyway.


Note that there are other options that are potentially cleaner from a "I should always avoid const_cast!" point of view. Personally, I think this is one of the legitimate uses of const_cast, and it is the approach I use. If you'd like to see more information on the other options, see the Stack Overflow post.

share|improve this answer
 
Hm I just reproduced the code from the top of my head and did not notice I had done it the other way around. Of course you are right. The non const code could do manipulations that we don't want the const code to suffer from. –  Nobody yesterday
add comment

First you have it the wrong way around.

You should not use the mutating version to supply data to the non-mutating function:

const int& MyClass::operator[](int index)const {
    return const_cast<int&>(const_cast<MyClass&>(*this)[index]);
}

What happens if operator[] is modified and does mutate slightly the internal state? By casting away the constness you have opened up your class to undefined behavior. You should NEVER cast away constness. If you must you can use const_cast to add constness, but removing it is so dangerous that this should be looked at as breaking code.

If you had written it the other way around:

int& MyClass::operator[](int index) {
    return const_cast<int&>(const_cast<MyClass const&>(*this)[index]);
}

This is better. Because if you change the operator[] so that it actually mutates the class it will generate a compiler error and thus prevent you from accidentally getting undefined behavior.

But personally I prefer to add another helper method and thus remove all athe casting:

private:
// NOTE: This is not a perfect solution for all situations.
//       But it works for 99% of situations and keeps the code clean
//       For the remaining situations switch back to use const cast
//       **BUT** always add never remove constness from an object.
// Never make public as it is const and gives away an internal reference.
// Only use via the public API
int& get(int index) const
{
    return stuff[index];
}

public:
// Keep the API simple/clean and DRY
// move all the work to private methods.
int&       operator[](int index)       { return get(index);}
int const& operator[](int index) const { return get(index);}

Answer to comment below:

I would not use it for const/non-const iterators

const_iterator begin() const {return const_iterator(<X>);}
iterator       begin()       {return iterator(<X>);}
share|improve this answer
 
Would you show how that works if get is supposed to return a const or non-const iterator type, instead of a const or non-reference? Perhaps get return a non-const iterator instance, and you define the const iterator type so that it can be explicitly or implicitly created from a non-const iterator instance? –  ChrisW yesterday
 
@ChrisW: Hope the edit helps. –  Loki Astari 22 hours ago
add comment

One (C++11) solution is to "hide" the ugly second const_cast with a macro (shudder):

#include <type_traits>
#define CONST_THIS const_cast< \
                      std::add_pointer< \
                          std::add_const< \
                              std::remove_pointer< \
                                  decltype(this)>::type>::type>::type>(this)

And use it like this:

#include <iostream>
struct Foo {
        void bar() {
                CONST_THIS->bar();
        }

        void bar() const {
                std::cout << "const called!\n";
        }
};

int main() {
        Foo().bar();
}
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.