C++ tips - Numerous class access functions

[ben2ong2]

PROBLEM: Mark Harrison (harrison@jho.com), 13 Jul 92

I have been looking at a class declaration which has a large number
of private data elements.  For every data element, there are two
public functions, one to set the value and one to get the value, i.e.
something like this:

   class X {
   private:
      int x;
      double y;
      ...
   public:
      void setx(int val) { x = val;  }
      int  getx(void)    { return x; }

      void   sety(double val) { y = val;  }
      double gety(void)       { return y; }
      ...
   }

Somehow, this doesn't seem "right", but I'm having trouble coming up
with a concrete reason.  Any comments?



RESPONSE: Bob Martin (rmartin@thor.Rational.COM)

Such constructs are not necessarily wrong, but they are often
indicative of a failure to think through the design of the class.  A
class is an abstraction whose internals are hidden from its clients.
By exposing all the variables with set/get functions, you are exposing
the internals of the class to its clients.

What should you do?  The methods of a class should express the
behavior of its objects.  Those methods operate on its instance
variables so as to implement the needed behaviors.  Consider, for
example, a CD player.  Such a class would have instance variables for
recording what track is currently playing, how much time is left on
the current track, how many tracks are on the current disk, etc.  But
these variables would not be exposed through 'set' functions to its
clients.  Instead, the clients must manipulate these variables
indirectly through methods like: Start, Stop, forward-track,
back-track, etc.

When designing a class, hide the instance variables from the clients,
as much as you can.  Try not to expose them directly, declare them
private, or protected.  Also try not to expose them indirectly through
get/set functions.  Instead, think about what the class is trying to
do, and expose interfaces which will allow the class to accomplish
those things.  If all you wind up with are get/set functions anyway,
then you may not have a true abstraction of the problem domain.
Perhaps you need to widen the abstraction until its embraces some real
functionality.