3

I'm trying to use const keyword correctly in my code. I have a class A that have some other classes B in and std::array as member variable.

I have an accessor to get one of A's B member.

  class B
  {
    public:
      int getMember() const;
  };


  class A
  {
    public:
      B& getB(const size_t idx)
      {
        return m_bCollection[idx];
      }

    private:
      std::array<B, 10> m_bCollection;
  };

Then I added a function to serialize an instance of A for yaml-cpp

  YAML::Emitter& operator<<(YAML::Emitter& out, const A& a)
  {
    // some stuff here, and then:
    out << a.getB(1).getMember();

    return out;
  }

This does not compile since the call to getB violates the const modifier for a argument of my serializing function.

error: passing ‘const A’ as ‘this’ argument discards qualifiers

I could overload my getB method to have a constant version of it, but this does not seems very clean...

  B& A::getB(const size_t idx);
  const B& A::getB(const size_t idx) const;

I have this issue on other similar classes as well and have const-overloaded a lot more method. This modification seems a mess to me. I think I missed a cleaner way to achieve my goal (having a const A& as serializer argument).

I only use const members of the non-const B instance reference returned by the const A instance. What should I refactor here to have clean code that follow c++ good practices ?

Nicolas Appriou
  • 1,457
  • 15
  • 32
  • 5
    `const B& A::getB(const size_t idx) const;`is the right way, the more you can put your members `const `the better it is. Note you can also have a `const` and a non `const` version of your members – bruno Jul 07 '20 at 08:39
  • 1
    This [post](https://stackoverflow.com/questions/123758/how-do-i-remove-code-duplication-between-similar-const-and-non-const-member-func) should be helping. It's also an item in Effective C++. Casting argument to const and cast away return value with const_cast is okay. – Louis Go Jul 07 '20 at 08:39
  • 5
    Having both `const` and not `const` overload *is* clean C++. – Yksisarvinen Jul 07 '20 at 08:41
  • Does this answer your question? [How do I remove code duplication between similar const and non-const member functions?](https://stackoverflow.com/questions/123758/how-do-i-remove-code-duplication-between-similar-const-and-non-const-member-func) – underscore_d Jul 07 '20 at 09:16

1 Answers1

2

I only use const members of the non-const B instance reference returned by the const A instance. What should I refactor here to have clean code that follow c++ good practices ?

The clean idomatic way is the one that you do not want because you consider it as messy. Your A should look like this

class A
  {
    public:
      B& getB(size_t idx)    
      {
        return m_bCollection[idx];
      }
      const B& getB(size_t idx) const {
        return m_bCollection[idx];
      }   
    private:
      std::array<B, 10> m_bCollection;
  };

To avoid code duplication you can use the solution presented here or other answers of that question.

In general you should make methods that do not modify members const by default. Only if you need them non-const supply a second overload (not the other way around). There will be many more cases where you will hit the same wall with your code. Just as one example, an overload for operator<< should have this signature:

std::ostream& operator<<(std::ostream&, const A&); 
                                       // ^--------  const !!!

Last but not least: I suppose it is due to simplification of your example, but your getA looks like an attempt for encapsulation. In fact it is not. Once you returned a non-const reference to a member you can make the member public as well. The only purpose getB serves is that you can write a.getB(index) instead of a.m_bCollection[index] but it does not achieve encapsulation.

463035818_is_not_a_number
  • 64,173
  • 8
  • 58
  • 126
  • 1
    I don't see how it's "dirty and hacky", at least not any more than having to copy and paste... In C++17 it already looks a lot better, and one can add a quick helper function to make it look better still: https://stackoverflow.com/a/47369227/2757035 – underscore_d Jul 07 '20 at 09:17
  • 1
    @underscore_d it was more hacky in my memory ;). Fixed that part – 463035818_is_not_a_number Jul 07 '20 at 09:19