4

Is it good practice to use template class objects with implicit getters and setters for attributes in (nearly) POD classes?

Consider the following template example:

template<typename T>
class Attribute
{
protected:
    T m_val;
public:
    T * getAddress() { return &m_val; }
    T get() { return m_val; }
    void set(T v) { m_val = v; }
};

and its usage:

class A
{
public:
    Attribute<float> floatAttr;
    Attribute<int> intAttr;
    Attribute<long> longAttr;
};

With this it is possible to have encapsulated data but with less implementation overhead.

Is this bad or good practice (and why)?

Edit: To state the advantages i see in this. There is no need to implement every getter setter function by hand, but there are still the usual advantages of these functions:

  • The data is encapsulated and the client needs to use the getter setter functions, which still can be implemented in a different way later on.
  • The internal usage is still hidden and can be changed.
  • Getter and Setter functions can be passed around as lambda functions.
kke
  • 353
  • 2
  • 8
  • 2
    What are the advantages of this vs. raw attributes? – Oliver Charlesworth Mar 28 '17 at 12:11
  • 2
    getters and setters are kind of the anti-pattern – NathanOliver Mar 28 '17 at 12:11
  • 3
    @Helion If you need just straight/simple getters and setters, than you probably do not need them, just make the member variable public. – roalz Mar 28 '17 at 12:14
  • Having both a setter and a getter like these seems redundant. Just let the user have access directly (public). Setters may, in general, perform range checking or other input validation. In such cases both a setter and a getter is fine. – Jonas Mar 28 '17 at 12:15
  • 2
    Possible duplicate of [Why use getters and setters?](http://stackoverflow.com/questions/1568091/why-use-getters-and-setters) – Borgleader Mar 28 '17 at 12:15
  • If you make generic functions then changing them later on is pointless anyway. – Hatted Rooster Mar 28 '17 at 12:26
  • This is pretty much the opposite of encapsulation. Also, your third point doesn't make much sense: there's no relation to lambdas here. – Quentin Mar 28 '17 at 12:26
  • @Quentin hiding a field value behind getter/setter functions is hardly "the opposite of encapsulation". – davmac Mar 28 '17 at 12:49
  • I feel like many of the commenters above have not understood the question, and perhaps in some cases the _raison d'etre_ for getter/setter methods: it's about the interface that is exposed to clients of the class, and how robust that interface when it comes to managing changes in the implementation. I believe the question is valid, even if it shows a limited understanding of C++. – davmac Mar 28 '17 at 12:53
  • 1
    @davmac encapsulation is about exposing an API that is simpler than the implementation. A one-to-one mapping that decorates each attribute with a pair of functions is not a simplification. That's why getter/setters have a bad reputation: many people think that littering your code with cumbersome function calls is encapsulation, and is somehow good. – Quentin Mar 28 '17 at 12:59
  • @Quentin, I disagree: encapsulation is about preventing implementation detail being exposed (and therefore fixed) in an interface. Getters and setters can help to achieve this (in some languages). OP's code also achieves this to some extent. From [Wikipedia](https://en.wikipedia.org/wiki/Encapsulation_(computer_programming)): _Encapsulation is used to hide the values or state of a structured data object inside a class_ – davmac Mar 28 '17 at 13:05
  • @davmac But there is no hiding here, at all. All three attributes are still in plain sight, only now you have to use `get()` and `set()` everywhere. – Quentin Mar 28 '17 at 13:17
  • @Quentin the "original" field values _are_ hidden, inside an object whose implementation (and even, potentially, type) can be changed. Hence, enapsulation, even if not a perfect form of it. – davmac Mar 28 '17 at 13:20
  • Geters and setters are an anti-pattern. Partly because of needless code complexity. But also because if you need to expose a field directly, it tends to be an indication that the class wasn't well designed. The code that uses the getter/setter should be incorporated into the methods. – Malcolm McLean Mar 28 '17 at 13:27

1 Answers1

5

In some other other languages, getters and setters are a way of preventing implementation detail escape into the interface; once you expose a field directly, you may not be able to later re-implement as a property (with getter and setter functions) without changing all sites in the code which access the field.

In C++, this doesn't (so strongly) apply. You can change the type of any field to a class which overrides operator=, and which converts to the required type implicitly (for the "get" side). (There are of course some uses where this doesn't apply; if a pointer or reference to the field is created in client code, for example - although I would personally avoid doing that and consider it dubious practice).

Also, because C++ is statically typed, it is also easier for tools (IDEs etc) to provide automatic refactoring, if you ever need to change a field and corresponding accesses into a getter/setter pair with appropriate calls.

In point of evidence, here is an alteration of your Attribute template which allows your "attribute" to act as if it were a directly-exposed field (except that & will return the address of the attribute rather than the hidden field):

template<typename T>
class Attribute
{
protected:
    T m_val;
public:
    operator T() { return m_val; }
    T &operator=(const T &a) { m_val = a; return m_val; }
};

If you really wanted to, you could also override operator&:

T *operator&() { return &m_val; }

... however doing so largely breaks encapsulation (and for that matter, you might consider changing the return type of operator= to T or void for the same reason).

If you had originally exposed the field directly, you could replace its definition with an instance of the above template, and most uses would be unaffected. This shows one reason why the getter/setter pattern is not always necessary in C++.

Your own solution, while it encapsulates the original field behind getter/setter functions, actually also exposes another field: the Attribute<T> member (floatAttr etc in your example). For this to work as a means of encapsulation, you rely on users not knowing (or caring) about the type of the attribute field itself; that is, you expect that no-one does:

A a;
Attribute<float> & float_attr = a.floatAttr;

Of course, if they do not do this and access the fields in the manner you intended, then it would indeed be possible to later change the implementation by altering the type of the "attribute" field:

A a;
float f = a.floatAttr.get();

... so in this sense, you achieve some encapsulation; the real issue is that there is a better way to do it. :)

Finally, it bears mentioning that both your proposed Attribute template as well as the alternative I show above both move the field into a class (Attribute<T> for some T) that is separate from the original parent class (A). If the implementation was to be changed, it is now limited to some degree by this fact; the attribute object does not naturally have a reference to the object that contains it. As an example, suppose I have a class B which has an attribute level:

class B {
    public:
    Attribute<int> level;
};

Now suppose I later add a "miminum level" field, min_level:

class B {
    public:
    Attribute<int> level;
    Attribute<int> min_level;
};

Further, suppose I now want to constrain level, on assignment, to the value of min_level. This will not be straightforward! Although I can give level a new type with a custom implementation, it will not be able to access the min_level value from the containing object:

class LevelAttribute {
    int m_val;
    public:
    T &operator=(const T &a) {
        m_val = std::max(min_level, a); // error - min_level not defined
    }
}

To get it to work, you'd need to pass the containing object into the LevelAttribute object, which means storing an extra pointer. The typical, old-fashioned setter, which is declared as a function directly in the class that holds the field, avoids this problem.

davmac
  • 20,150
  • 1
  • 40
  • 68