Importance of Accessor/Mutator methods

[humble_learner]

Hi,

We have a project which contains a number of 'Entity' classes - classes which hold data to be passed around. As we are dealing with C++, is it a rule that Get() and Set() methods are necessary for all the private data members ?

It sometimes seems to be an overheard having these methods but in the interests of OOAD, is it a rule that data should be private and hence accessed only using the Set()/Get() implementations ?

Ofcourse C++ texts would insist on the Set/Get() implementations, but in real life projects with a number of simple data types in a class (entity class), is it necessary for the accessor and mutators ?

I would personally do away with Set() and Get() because the entity classes by themselves are so simple that can be replicated in structures too.

Please let me know your thoughts.

[TheCPUWizard]

ANY statemet that includes "ALWAYS" or "NEVER" is ALWAYS wrong.

However, Get/Set is a good idea in 99.999% of the cases.

If they are implemented as inline methods, then there is (almost always) no overhead at runtime (the "typing" overhead does NOT EVER count).

Yes your class is simple TODAY.

But next week....

You decide to lazy evaluate a gettable piece of data....
You need to add a validation to a settable peiece of data...
You decide to agrssively evaluate a peice of getabble data in the set of a different piece....

All three of these patterns (and more) occur on a regular basis. If you exposed the "naked" data, then not only do you have to change your class, but you also have to change all of the places where your class is used.

See the ramifications????

[Kheun]

It is not necessary to provide a pair of accessor and mutator functions for every private member variable. As long as there isn't any need, you don't have to provide them.

[TheCPUWizard]

It is not necessary to provide a pair of accessor and mutator functions for every private member variable. As long as there isn't any need, you don't have to provide them.

One of us mis-read the original question.

Complete agreement that you should not expost every private member. That would be plain silly..

I took the question to mean..if there is a peice of thata that you MUST expose, should it be implemented as "Get/Set with private data" or simply expose the data member publically....

[Kheun]

Yeah, either that or the question isn't clear. Either way, both advices are correct.

[GNiewerth]

Accessor/Mutator functions define the interface of a class and hide the implementation. By using accessor/mutator functions the caller doesn´t have to know how or where data is stored, that´s left to the callee. When correctly implemented the callee code can be freely modified, as long as the callee´s interface remains untouched.

Take a look at this example:

#include <vector>

class Dataset
{
   std::vector<double> m_Data;

public:
   Dataset() : Average( 0.0 )
   {
   }

   void update( double dData )
   {
      // store data in vector
      m_Data.push_back( dData );

      // update average (calculation on previous average)
      // separated into 3 steps for readability reasons
      Average = (Average * m_Data.size() -1);
      Average += dData;
      Average /= m_Data.size();
   }
   double Average;
};

Now you can use this class and call update to store data in it. Each time you update data the average will be automatically calculated and you can fetch it from the Average member.

After a while you notice you need to call update() more often than you need to access Average, so you decide you don´t want to calculate the average with each update() call and you end up with this:

#include <vector>
#include <numeric>

class Dataset
{
   std::vector<double> m_Data;

public:
   Dataset();

   void update( double dData )
   {
      // store data in vector
      m_Data.push_back( dData );
   }

   void calculate_average()
   {
      if( false == m_Data.empty() )
      {
         // compute average of stored values
         Average = accumulate( m_Data.begin(), m_Data.end(), 0.0 );
         Average /= m_Data.size();
      }
      else
      {
         // there are no values, return 0
         Average = 0.0;
      }
   }     
   double Average;
};

Now you can call update() as often as you want without having the average calculated with each call. The drawback, however, is you have to call calculate_average before you access Average, so you have to adjust you current code to call calculate_average before accessing Average. Doh.

If you had used an accessor function instead of directly accessing the Average member your class would have evolved from

#include <vector>

class Dataset
{
    double m_dAverage; 
   std::vector<double> m_Data;

public:
   Dataset()  : m_dAverage( 0.0 )
   {
   }

   void update( double dData )
   {
      // store data in vector
      m_Data.push_back( dData );

      // update average (calculation on previous average)
      // separated into 3 steps for readability reasons
      Average = (Average * m_Data.size() -1);
      Average += dData;
      Average /= m_Data.size();
   }

    double get_average() const
   {
      return m_dAverage;
   } 
};

to

class Dataset
{
   std::vector<double> m_Data;

public:
   Dataset()
   {
   }

   void update( double dData )
   {
      // store data in vector
      m_Data.push_back( dData );
   }

    double get_average() const
   {
      double dAverage = 0.0;
      if( false == m_Data.empty() )
      {
         // compute average of stored values
         Average = accumulate( m_Data.begin(), m_Data.end(), 0.0 );
         Average /= m_Data.size();
      }
      return dAverage;
   } 
};

Aaaaah, much nicer now. The class´ interface has not been changed, you can still call get_average() without modifying existing code.
For some reasons there was a change in the client code, it now calls get_average() significantly more often than it calls update(), so repeated average calculations slows down the application. You may come up with this implementation to fix that issue:

#include <vector>
#include <numeric>

class Dataset
{
    double m_dAverage;
   bool     m_bAverageDirty; 
   std::vector<double> m_Data;

public:
   Dataset() : m_dAverage( 0.0 ), m_bAverageDirty( true )
   {
   }

   void update( double dData )
   {
      // store data in vector
      m_Data.push_back( dData );

      // Data has been modified, Average is no longer valid

      m_bAverageDirty = true;
   }

    double get_average() const
   {
      if( true == m_bAverageDirty )
      {
         // Average needs to be calculated
         m_dAverage = accumulate( m_Data.begin(), m_Data.end(), 0.0 );
         m_dAverage /= m_Data.size();

         // Average is valid now
         m_bAverageDirty = false;
      }
      return m_dAverage;
   } 
};

Again the interface of the class has not been changed, though there have been major changes in the implementation.

For objects with up to 10 simple members and no further functionality besides grouping data I don´t use accessor/mutator functions.

[TheCPUWizard]

And excellent example of agressive and lazy evaluation.

[_uj]

Please let me know your thoughts.

There are different kinds of classes. Some classes are close to primitives in nature. Examples of such classes are say Complex (abstracting a complex number (two floats)), and Tuple3D (abstracting a 3D coordinate (three floats)). In those cases setters/getters is to overdo it.

At some point a class becomes a more involved data abstraction and then one can start thinking about exposing private variables using getters/setters. But note that getters/setters are a very rudimentary form of encapsulation. A much better and stronger form is to not expose the internal state of a class at all. This is accomplished by asking the class to perform stuff, rather than shuffling data in and out of it.

[TheCPUWizard]

There are different kinds of classes. Some classes are close to primitives in nature. Examples of such classes are say Complex (abstracting a complex number (two floats)), and Tuple3D (abstracting a 3D coordinate (three floats)). In those cases setters/getters is to overdo it.

#1.. Many design choices are personal. What I am 100% convinced of is that a consistant approach is better than an inconsistent approach.

#2..Regarding overkill. I have to disagree (now it gets personal). Even for trivial cases, I find that the potential future problems outweights the typing overhead (the compiler will almost certainly optimize the trivial get/set out of existing resulting in the code actually executing direct access to the data - so there is no runtime (memory or speed) issue.

Consider what happens if you have a complex class and you expose the two floats. Users of this class can take the address of the members. Now you decide to do all your complex math in doubles, but still expose it as floats (not uncommon to use a higher resolution for internal calculations). Even if your compiler supported "property syntax extensions", you are still in big trouble as your client call will not fail horribly (ie not compile!).

If someone could provide a single example where there is a downside to exposing the data (again typing does NOT count)I would be very interested in seeing it.

In 28 years of programming in C/C++, I have NEVER had to edit a file to change a get/set to an exposed data member. However there have been many times I have had to do the reverse (in other peoples code). From a code reuse (no-edits / no-recompiles) scenario any change to a "completed" item is indicitive of a bug/flaw in the original design.

btw: How many people (that know how to set a breakpoint when a statement executes) do not know how to set a data write breakpoint?? And even if you DO know, nearly every (non-ICE) debugger will impose a serious performance impact on such a breakpoint.

[kempofighter]

Yeah, either that or the question isn't clear. Either way, both advices are correct.

In the OP, private data was mentioned. I took that too mean, is it necessary to provide the accessor/mutator for private attributes (as opposed to not providing accessor mutators for private attributes). It didn't seem like the OP was asking whether to expose public data. However, defining mutators for private attributes is not much better than making them public in my opinion. n fact, mutators should not be provided unless there is some valid reason to allow a user to modify the state of the class. Many variables might represent a critical state and allowing outside users to modify the state could be bad.

One reason that it might make sense to define the accessor is for testability. Since private data can't be accessed by test drivers, it might make sense to have them so that test drivers can always test the internal state of the class without having to mess with the friend mechanism. So you might want to think about how you are planning to unit test your code, if at all. I can't think of any other reason to provide accessors for all attributes, by default. Don't define them just for the sake of defining them. Tools can be set to not generate them.

[TheCPUWizard]

One reason that it might make sense to define the accessor is for testability. Since private data can't be accessed by test drivers, it might make sense to have them so that test drivers can always test the internal state of the class without having to mess with the friend mechanism. So you might want to think about how you are planning to unit test your code, if at all. I can't think of any other reason to provide accessors for all attributes, by default. Don't define them just for the sake of defining them. Tools can be set to not generate them.

While that is certianly one way to address the testability issue. I will typically take it a step further to minimize "abuse", IF you utilize Factory type patterns to support a DI architecture...

If you need access to provate data for testing putposes, implement a PROTECTED accessor with a specific name (e.g Get_XXX_4Test()). Then implement a derived class. By using a specialized factory, you transparently create your test instances where the test code can access the information in question, but there is no risk that the "production" code will be developed against the "Test" interface.

If you do NOT utilize a DI ready architecture, then creating an INDEPENDANT class which can contain a "mirror" of the state can help. In this pattern you simply ad one method that returns you a snapshot of the state (ie an instance of the INDEPPENDANT class).

In any case, the goal is always to expose the minimum amount of information that is absolutely required.

[active2volcano]

In our <<C++ programming Criterion>>, all the data must be private!
I am also accept the viewpoint of TheCPUWizard!

[_uj]

#2..Regarding overkill. I have to disagree (now it gets personal). Even for trivial cases, I find that the potential future problems outweights the typing overhead (the compiler will almost certainly optimize the trivial get/set out of existing resulting in the code actually executing direct access to the data - so there is no runtime (memory or speed) issue.

Well, What about int?

Is it a class or is it not?

[pm_kirkham]

If you are using C++ to do object-flow programming (a fudge between imperative data modelling and OOP, where you pass an object as a bag of parameters then change its state), then accessors/mutators are a mechanism to abstract away the data storage implementation from the schema.

If you're using C++ to do OOP (where you send a message to an object in order to request a behaviour), then you don't expose any state externally at all.

Usually if you find you need to access state, it's because you have an object which represents an entity rather than a set of associated behaviours. In those cases, I often find it's better to use demeter-style visitors for the non-identifying attributes, rather than getters.

Setters are usually a bad idea in any case, as you rarely want to just set a value without wanting a change in the behaviour of the object, and you often have co-constraints between values, or the ordering in which values are set, which simple setters don't enforce. Getters often result in moving brittleness from the implementation of an attribute to a data schema and encourage a procedural, navigation-oriented style. Which is OK for little scripts for automating COM components (COM and Java beans both tend to be very navigation oriented), but is brittle in larger projects.

[active2volcano]

Could you explain "navigation-oriented style" in detail?