Const C++ DRY Strategies

Question

For avoiding non-trivial C++ const related duplication, are there cases where const_cast would work but a private const function returning non-const wouldn't?

In Scott Meyers' Effective C++ item 3, he suggests that a const_cast combined with a static cast can be an effective and safe way to avoid duplicate code, e.g.

const void* Bar::bar(int i) const
{
  ...
  return variableResultingFromNonTrivialDotDotDotCode;
}
void* Bar::bar(int i)
{
  return const_cast<void*>(static_cast<const Bar*>(this)->bar(i));
}

Meyers goes on to explain that having the const function call the non-const function is dangerous.

The code below is a counter-example showing:

• contrary to Meyers's suggestion, sometimes the const_cast combined with a static cast is dangerous
• sometimes having the const function call the non-const is less dangerous
• sometimes both ways using a const_cast hide potentially useful compiler errors
• avoiding a const_cast and having an additional const private member returning a non-const is another option

Are either of the const_cast strategies of avoiding code duplication considered good practice? Would you prefer the private method strategy instead? Are there cases where const_cast would work but a private method wouldn't? Are there other options (besides duplication)?

My concern with the const_cast strategies is that even if the code is correct when written, later on during maintenance the code could become incorrect and the const_cast would hide a useful compiler error. It seems like a common private function is generally safer.

class Foo
{
  public:
    Foo(const LongLived& constLongLived, LongLived& mutableLongLived)
    : mConstLongLived(constLongLived), mMutableLongLived(mutableLongLived)
    {}

    // case A: we shouldn't ever be allowed to return a non-const reference to something we only have a const reference to

    // const_cast prevents a useful compiler error
    const LongLived& GetA1() const { return mConstLongLived; }
    LongLived& GetA1()
    {
      return const_cast<LongLived&>( static_cast<const Foo*>(this)->GetA1() );
    }

    /* gives useful compiler error
    LongLived& GetA2()
    {
      return mConstLongLived; // error: invalid initialization of reference of type 'LongLived&' from expression of type 'const LongLived'
    }
    const LongLived& GetA2() const { return const_cast<Foo*>(this)->GetA2(); }
    */

    // case B: imagine we are using the convention that const means thread-safe, and we would prefer to re-calculate than lock the cache, then GetB0 might be correct:

    int GetB0(int i) { return mCache.Nth(i); }
    int GetB0(int i) const { return Fibonachi().Nth(i); }

    /* gives useful compiler error
    int GetB1(int i) const { return mCache.Nth(i); } // error: passing 'const Fibonachi' as 'this' argument of 'int Fibonachi::Nth(int)' discards qualifiers
    int GetB1(int i)
    {
      return static_cast<const Foo*>(this)->GetB1(i);
    }*/

    // const_cast prevents a useful compiler error
    int GetB2(int i) { return mCache.Nth(i); }
    int GetB2(int i) const { return const_cast<Foo*>(this)->GetB2(i); }

    // case C: calling a private const member that returns non-const seems like generally the way to go

    LongLived& GetC1() { return GetC1Private(); }
    const LongLived& GetC1() const { return GetC1Private(); }

  private:
    LongLived& GetC1Private() const { /* pretend a bunch of lines of code instead of just returning a single variable*/ return mMutableLongLived; }

    const LongLived& mConstLongLived;
    LongLived& mMutableLongLived;
    Fibonachi mCache;
};

class Fibonachi
{ 
    public:
      Fibonachi()
      {
        mCache.push_back(0);
        mCache.push_back(1);
      }

      int Nth(int n) 
      {
        for (int i=mCache.size(); i <= n; ++i)
        {
            mCache.push_back(mCache[i-1] + mCache[i-2]);
        }
        return mCache[n];
      }

      int Nth(int n) const
      {
          return n < mCache.size() ? mCache[n] : -1;
      }
    private:
      std::vector<int> mCache;
};

class LongLived {};

Answer 1

When implementing const and non-const member functions that only differ by whether the returned ptr/reference is const, the best DRY strategy is to:

1. if writing an accessor, consider whether you really need the accessor at all, see cmaster's answer and http://c2.com/cgi/wiki?AccessorsAreEvil
2. just duplicate the code if it is trivial (e.g. just returning a member)
3. never use a const_cast to avoid const related duplication
4. to avoid non-trivial duplication, use a private const function returning a non-const that both the const and non-const public functions call

e.g.

public:
  LongLived& GetC1() { return GetC1Private(); }
  const LongLived& GetC1() const { return GetC1Private(); }
private:
  LongLived& GetC1Private() const { /* non-trivial DRY logic here */ }

Let's call this the private const function returning non-const pattern.

This is the best strategy for avoiding duplications in a straight forward fashion while still allowing the compiler to perform potentially useful checks and report const related error messages.

Answer 2

Yes, you are right: Many C++ programs that attempt const-correctness are in stark violation of the DRY principle, and even the private member returning non-const is a bit too much complexity for comfort.

However, you miss one observation: code duplication due to const-correctness is only ever a problem if you are giving other code access to your data members. This in itself is in violation of encapsulation. Generally, this kind of code duplication occurs mostly in simple accessors (after all, you are handing access to already existing members, the return value is generally not the result of a calculation).

My experience is that good abstractions do not tend to include accessors. Consequently, I largely avoid this problem by defining member functions that actually do something, rather than just providing access to data members; I try to model behavior instead of data. My main intention in this is to actually get some abstraction out of both my classes and of their individual member functions, instead of just using my objects as data containers. But this style is also quite successful in avoiding the tons of const/non-const repetitive one-line accessors that are so common in most codes.