Should a class use its private members directly or its publicly exposed member functions?

Question

Well, I'm not sure about this one.

I have a class like

    class Object
    {
        int internalValue ;
        struct SomeStructType
        {
            int superInternalValue ;
        } someStruct ;

    public:
        int getInternalValue()
        {
            return internalValue ;
        }

        int getSuperInternalValue()
        {
            return someStruct.superInternalValue ;
        }

        void printThatInternalValue()
        {
            // Its seems pretty clear we should just access
            // the private value directly and not use the public getter
            printf( "%d", internalValue ) ;
        }

        void printThatSuperInternalValue()
        {
            // Now here the access pattern is getting more complicated.
            // Shouldn't we just call the public access function
            // getSuperInternalValue() instead, even though we are inside the class?
            printf( "%d", someStruct.superInternalValue ) ;
        }
    } ;

So for the class's INTERNAL operations, it CAN use someStruct.superInternalValue directly, but it seems cleaner to use the class's public getter function getSuperInternalValue() instead.

The only drawback I can see is if you tried to modify superInternalValue using the getter would give you a copy, which is clearly not what you want in that case. But for reading access, should the public getter function be used internally inside a class?

Answer 1

If the accessor cannot be overridden, a class can use its private members directly. Otherwise, it should use the accessor, so that if it is overridden, the class will continue to operate correctly.

Answer 2

I would suggest that, if in doubt, use the accessor.

While an accessor may, originally, act as a simple wrapper that does nothing more than gets the value of the private member, a future modification to the accessor may change this behaviour such that any other method accessing the member directly no longer has a consistent view of its value.

Answer 3

The only other reason I can see to use the public accessor is just in case the accessor does something other then just returning the value saved in the variable; for example, returning a default value if the variable in memory is invalid for some reason.

In the example you gave, either way is fine, and accessing the variable directly means less code for your program to go through to get the value to show.

Answer 4

It all depends. I don't think that in general you should use getter inside class. For example,

int MyClass::getFoo() const
{
   int retval(0);
   m_myMutex.lock();
   retval = m_foo;
   m_myMutex.unlock();
   return retval;
}

You won't probably use such a getter inside MyClass.

Answer 5

I'd say at least 90% of the time that questions like this arise, it's a good sign that you're probably trying to put too much functionality into one class. If someStruct::SuperInternalValue should really be accessed via getSuperInternalValue, then chances are pretty good that it should be part of someStruct instead of being part of Object.

Likewise, if you need to print out a someStruct, then Object should just use a member function (or overloaded operator in a language that supports it) to print out a someStruct object (as a whole). As you've written it, Object::printThatSuperInternalValue knows everything about someStruct and superInternalValue.

Despite source code containing "class" and "Object", what you really have is code in Object acting on dumb data in someStruct. In other words, what you have isn't really OO at all -- it's simple procedural code with a data structure. That's not necessarily all bad, but based on the encapsulation tag, I think it's a fair guess that it's probably not what you really want.

class Object { 
    int internalValue;
public:
    class someStruct { 
        int superInternalValue;
    public:
        someStruct(int v) : superInternalValue(v) {}
        someStruct &operator=(someStruct const &n) { 
             superInternalValue = n.superInternalValue;
             return *this; 
        }
        friend ostream &operator<<(ostream &os, someStruct const &s) {
            return os << s.superInternalValue;
        }
    };
    friend ostream &operator<<(ostream &os, Object const &o) { 
        return os << internalValue;
    }
};

This way we've defined someStruct in terms of an interface (we can create one from an int, assign them and print them) and Object doesn't need to deal with/touch superInternalValue at all. The important part isn't that we've stopped Object from playing with the internals of somestruct, but that we've made someStruct intelligent enough that Object can now ignore all the gory details of how someStruct is implemented.