10

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 {};
  • A getter that just returns a member is shorter than one that casts and calls the other version of itself. The trick is meant for more complicated functions where the gain of deduplication outweighs the risks of casting. – Sebastian Redl Jul 15 '15 at 18:52
  • @SebastianRedl I agree that duplication would be better if just returning member. Please imagine that it is more complicated, e.g. instead of returning mConstLongLived, we could call a function on mConstLongLived that returns a const reference which then is used to call another function that returns a const reference which we don't own and only have access to a const version of. I hope the point is clear that the const_cast can remove const from something that we wouldn't otherwise have non-const access to. – JDiMatteo Jul 15 '15 at 19:13
  • 4
    This all seems sort of ridiculous with simple examples, but const related duplication comes up in real code, const compiler errors are useful in practice (often for catching stupid mistakes), and I am surprised that the proposed "effective C++" solution is a strange and seemingly error-prone pair of casts. A private const member returning a non-const seems clearly superior to a double cast, and I want to know if there is something I'm missing. – JDiMatteo Jul 15 '15 at 19:20
5

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 */ }

This is called 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.

1

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.

  • It seems up for debate whether or not accessors are good, e.g. see the discussion at c2.com/cgi/wiki?AccessorsAreEvil . In practice, regardless of what you think of accessors, large code bases often use them, and if they do use them it would be better to adhere to the DRY principle. So I think the question deserves more of an answer than that you shouldn't ask it. – JDiMatteo Jul 16 '15 at 22:27
  • 1
    It's definitely a question worth asking :-) And I won't even deny that you need accessors from time to time. I'm merely saying that a programming style that's not based on accessors greatly reduces the problem. It does not solve the problem altogether, but it's at least good enough for me. – cmaster Jul 17 '15 at 5:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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