Should I avoid making private field variable public for convenience?

user_10580175 0
Viewed 83

I have two objects, A and B. Right now, B stores A as a local class variable.

class B
{
    A myA;
}

Now, B has a bunch of methods which uses its inner myA to do some stuff, and I can use those methods since they are public.

But sometimes, I need to use the myA itself. So what I did was to make it public, and then I could write myB.myA.someMethodFromA().

Is this okay, or is it bad style? I mean, I know that I could just provide indirect accessors to myA via methods in the B class, but that seems unnecessary when I can just directly access myA.

For example, if myA has a method doStuffviaA, I'd rather say myB.myA.doStuffViaA(), than first having to write a method in B that says

void doStuffViaB() { myA.doStuffViaB() }

But, of course making myA public means that it can be changed without B knowing about it. WHAT TO DO?

design-patterns c++
3 Answers

Yes, making it public circumvents encapsulation and code can creep in that puts your object into an ill-defined state.

A half-way house option would be to supply

const A& getMyA() const
{
    return myA;
}

since then, at least, only const functions can be performed on the myA member if accessed using this function. I'm hoping, of course, that doStuffViaA is const, then you'd write

myB.getMyA().someMethodFromA();
user_23808300

WHAT TO DO?

It depends and is in fact quite dependent on the specific scenario. Generally, I'd advice you to

  • use public data members within plain, invariant-free structs only (C2)
  • use a class once you have some invariant, make data members private (C2, C8)
  • avoid getters if possible.

If the latter is not possible (e.g. due to "design constraints" or a burning desire for "convenience"), you can provide a getter as posted in @Bathsheba's answer. Alternatively, you can pass a pointer to an A member function or a function accepting an A& parameter to a member function template of B with associated function parameter, e.g.

#include <functional>

class B {
   public:
      template <class MemberFct, class ...Args>
      decltype(auto) invokeOnA(MemberFct fct, Args&&... args)
      {
         return std::invoke(fct, a, std::forward<Args>(args)...);
      }

   private:
      A a;
};

Note that this requires C++17. Given a member function A::doStuff(int), this template can be instantiated and invoked with

B b;

b.invokeOnA(&A::doStuff, 42);
b.invokeOnA([](A& a){ a.doStuff(42); });

which solves the problem by another level of indirection and without a getter method. Note, however, that the approach is qualitatively very similar to providing a getter method, i.e., having a fancy function template doesn't free you from a justification why it's necessary to break encapsulation in the specific use case.

user_95935960

I think you already hit the crucial point:

But, of course making myA public means that it can be changed without B knowing about it. WHAT TO DO?

Making a member public or private is not just about convenience. Ask yourself: Are there invariants that my class relies on that can break if the member is public?

Only if you can definitely answer this with "no" you can make a member public. However, in that case you could ask: Why is it a member of the class anyhow?

Consider this simple broken example:

struct foo { 
    int a,b,sum;
    foo(int a,int b) : a(a), b(b), sum(a+b) {}
};

To ensure that sum == a+b always holds, none of the members should be public. On the other hand, consider std::pair, which is just a data structure, so having first and second public is fine.

user_41177280
Related