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 ?

4
  • 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 Commented Jul 7, 2020 at 8:39
  • 1
    This post 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. Commented Jul 7, 2020 at 8:39
  • 5
    Having both const and not const overload is clean C++. Commented Jul 7, 2020 at 8:41
  • Does this answer your question? How do I remove code duplication between similar const and non-const member functions? Commented Jul 7, 2020 at 9:16

1 Answer 1

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.

Sign up to request clarification or add additional context in comments.

2 Comments

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: stackoverflow.com/a/47369227/2757035
@underscore_d it was more hacky in my memory ;). Fixed that part

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.