Excessive Encapsulation Syndrome

I have seen this occasionally on other code bases, where an object’s attribute is declared private, and then both a getter and a setter is provided, effectively exposing the attribute as public. For example

class Foo
{
   int bar;
public:
   int getBar() const {return bar;}
   void setBar(int x) {bar=x;}
};

I take this as a code smell that bar should have been declared public in the first place. Requiring the use of accessors means that the code is littered with foo.getBar() or foo.setBar(2.0), which is arguably less readable than the foo.bar equivalent, and it also means there is now useless code that has been added, and Code That Doesn’t Exist Is The Code You Don’t Need To Debug.

Don’t get me wrong, I’m not arguing that encapsulation is evil. If you have ever written C code, which doesn’t have encapsulation, you will realise it is an enormously valuable technique. To understand when encapsulation should be used, you need to understand the concept of an invariant. An invariant is a condition about the class that must hold at all times. Consider the example of a simple vector class, which we implement in terms of a pointer and a size attribute:

struct Vec
{
   double *data=nullptr;
   size_t size;
   Vec(size_t size=0): size(size) {data=new double[size];}
   ~Vec() {delete [] data;}
};

In this code, we have taken care of memory management issues by using RAII, however what happens if a user wants to resize the vector by assigning a newly allocated array to the data pointer. Maybe they might use the malloc() function for the purpose, or they might simply take the address of some object on the stack:

double a[]={1.0,2.0};
Vec b;
b.data=&a;

These usages will lead to insidious memory leaks, or outright crashes if you’re lucky. There are a couple of implicit invariants here: One is that the data member is a heap allocated array by the new[] operator, and the second that size is less than or equal to the size of the allocated array.
In this case, encapsulation is entirely appropriate:

class Vec
{
   double *data;
   size_t m_size;
public:
   Vec(size_t size=0): size(size) {data=new double[size];}
   ~Vec() {delete [] data;}
   size_t size() const {return m_size;}
   size_t size(size_t newSize) {
      if (newSize>m_size) {
        delete [] data;
        data = new double[newSize];
      }
      m_size=newSize;
   }
};

Of course, there is another invariant here, and that is that one and only one Vec object exists for each allocated heap object. That invariant is violated by the implicit copy constructor and assignment operator. At a minimum, these methods should be deleted, but alternatives such as explicitly creating a new data pointer and copying the contents is also possible.

What, then, if there were no invariants that need to be preserved by a particular member? Is there any other reason to use encapsulation? The only other reason I can think of is if you’re writing a library (an API), and you wish to insulate users of that library against changes to the implementation. For example, in the original Foo example above, you may decide one day that bar should be a computed value from some other items, eg multiplying two values. Then your change will break any code that links against your library. But there are two answers to this: if you are creating a library for others, you should be versioning your library, and such breaking API changes should be reason for a major version bump. On the other hand, if the class is for internal use only, then you just need to refactor your code to handle the new interface. For even quite large code bases (eg 1 million lines), that rarely takes more than a few hours. So I would argue that unless you foresee a likelihood for an implementation change, encapsulating members for that reason is premature pessimisation. YAGNI!

Anyway, I did wonder why in some code bases there are private member variables, with useless public setters and getters. Recently I joined a group who shall remain nameless that has as its coding style “all data members should be private” (with the exception for POD structures where everything is public). Obviously, under such a coding style, useless accessor methods will proliferate, as it is the only allowed way to implement a public attribute. I don’t know how widespread this belief that encapsulation is so good it should be used everywhere, even when it has no functional benefit. That encouraged me to write this hopefully incendiary post. Please leave comments!

Did you like this? Share it:
This entry was posted in Uncategorized and tagged , , . Bookmark the permalink.

Leave a Reply