Is encapsulation via public const pointers a good idea?

Question

So here is what I know:

• It is wise not expose your ivars directly in your API; rather, use accessors
• A const pointer to a non-const object simply means you can alter the object, but not redirect where the pointer points

Here is my situation:

I have a few related classes. I want to create a simple class that, via composition, combines these into one logical interface. Each of my enclosed classes already has a public and private distinction in its API so I do not mind exposing them directly to users of my parent class. This means that it would be overkill for me to write accessors for these ivars since the classes already manage what is public and what isn't. But, I don't want users to change the actual objects that are enclosed into this composed parent class.

So the only way I can think to do this is to use const pointers to these objects, as such:

class Parent{

  public:

   EnclosedClass1*const ec1; //users can enjoy the public API of EnclosedClass
   EnclosedClass2*const ec2;

   void convenienceMethod(){
         //performs inter-related functions on both ec1 and ec2
    }

 }

This way, there is no harm in letting someone directly interface with the API of ec1 and ec2 but, I want to make sure this is fairly foolproof in that at least that person cannot change the actual resource being manipulated, hence the const pointers.

Does this make sense, is it a good use of const pointers?

Alternately, I could make them private entirely, forget the pointers (this class manages these objects anyway), and just do the extra work to write accessors for the public functions contained in these objects. But that just seems overkill, right?

Answer 1

The traditionnal OOP way is to keep data private, and have public accessors if one want to use them outside of the class.

If you don't want the private data pointers to be modified, then you don't provide setters, only the getters.

class Parent{

  private:
       EnclosedClass1*const ec1;
       EnclosedClass2*const ec2;

  public:
      EnclosedClass1* getEnclosedClass1() const {...};
      EnclosedClass2* getEnclosedClass2() const {...};    

      void convenienceMethod(){
    }

}

another possibility (like @pmr proposes in the comments below) is:

const EnclosedClass1& getEnclosedClass1() const {...};
const EnclosedClass2& getEnclosedClass2() const {...};    

with const reference you protect the user from testing the value returned by the getters. But you must ensure that your object state is consistent and never have internal pointers to nullptr; in such case you should throw an exception.

Answer 2

There are different things wrong with this approach. The first one is that the fact that the members have a public and a private side to their interface, does not mean anything from the point of view of the enclosing type. The complete type can have a different set of invariants that users could break by independently modifying the two subobjects.

Even if the Parent type does not have any invariant (i.e. it does not depend on the state of the subobjects), the proposed approach is not const-correct. Given a const Parent& the caller can apply any operation (const and not const) to both subobjects, effectively changing the state of the Parent object. A slightly better approach in this case would be to store the subobjects directly:

class Parent {
public:
   EnclosedClass1 ec1;
   EnclosedClass2 ec2;
...

But I would recommend that you do provide accessors to extract the data and mutators (if the word makes sense) to change the state of the objects. That way if you need to add invariants later on you can just stick them there.

Answer 3

Does this make sense ? Yes.

Is it a good use of const pointers ? Well, not really. The canonical way of coding is to have a getter function for each element and making internal data private. Doing another way will make readers of your code wonder why you're doing that since it is not standard practice.

So make your const pointers private and create getters to access them. For performance reasons, you can make these getters inlined functions, so that won't make any difference in the generated executable but will greatly enhance the readability of your code.

Answer 4

If you have a class that contains two other objects and provides free and full access to those members to its clients, then you should think about whether your class has the right design. The law of demeter doesn't become invalid because you don't use a getter, i.e. parent.ec1->foo is no better than parent.getEc1().foo.

In any case, const members are generally more trouble than they're worth.

Answer 5

one of the reason could be that it could be transform to non const pointer by constcast.

Answer 6

Decide what your class is supposed to do, and write an interface that provides those things. If it's important to allow users to mess with internal data, then write interfaces to do that. However, in most cases, a class presents an abstraction that's different from the sum of the things you can do with its internal data, so the interface should not expose internal data. For example, a class that holds names of students will typically implement that as some sort of container that holds std::string data. That class should not provide a pointer or reference to the string for a person's name. Instead, if a name needs to be corrected, for example, that should be done in a separate editor class; once corrected, the stored name can be updated with the new version. There's no need for the class to expose the entire std::string interface to users.