C++ – Const DRY Strategies

cconstdry

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 {};

Best Answer

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.

Related Topic