Using value wrapper and operator() overloading to simplify getter/setter design : a dangerous practice?

Question

Consider the following class:

class MyClass1
{
    public:
        double x() const {return _x;} // getter
        double y() const {return _y;} // getter
        double z() const {return _x*_y;} // getter
        void x(const double var) {_x = var;} // setter
        void y(const double var) {_y = var;} // setter
        void z(const double var) {_x = var; _y = 1;} // setter
    protected:
        double _x;
        double _y;
};

As the actual contents of MyClass1 is an implementation detail, the getters and setters provide a unified way to get and set the class contents even if they are interdependant (here _z does not exists internally but for the user, z is a variable like x and y).

Now to avoid to have to write getter/setter for x and y, one can use a wrapper like this:

template <typename Type>
class Wrapper
{
    public:
        constexpr Wrapper(const Type& value) {_value = value;}
        constexpr Type& operator()() {return _value;}
        constexpr const Type& operator()() const {return _value;}
        constexpr void operator()(const Type& value) {_value = value;}
    protected:
        _value;
};

And now the original class becomes:

class MyClass2
{
    public:
        Wrapper<double> x;
        Wrapper<double> y;
        double z() const {return x*y;} // getter
        void z(const double var) {x = var; y = 1;} // setter
};

Is it a dangerous practice or a good solution to avoid to have to write getters/setters ?

Note : Here MyClass1 and MyClass2 are just examples. My question is very "general" : is it dangerous to replace getters/setters of classes by the proposed Wrapper when the getter/setter just return/set an internal value.

Flannery

I suppose you would need to implement some kind of behavior pattern to actually do something (validate, notify, etc) on the accessors. Othewise they would defeat the purpose of having them. Don't you think? imho – Babs

why does your class need get/set interface? if these functions do not have to maintain a class invariant, there is little point in encapulating them. – Invasion

Answer 1

As you mentioned, the main purpose of getters and setters are to provide a unified way of accessing and setting your private instance variables.

From your wrapper solution, if sometime down the track of program lifecycle you decide to change a setter, you can just throw away the wrapper and replace it with the original getter / setter like in MyClass1 -- without having to change all other components of the code that calls it.

So from that point I think your solution is a valid way of saving you from typing extra few lines of code.

Surfeit

Answer 2

I see nothing particularly dangerous there, but it doesn't seem to gain anything. C++ provides reference semantics for any kind of object, which is incompatible with a setter function. Sometimes the actual contents are not just a detail.

You can also go the other way (actually, this is what I expected to see when I clicked this question):

struct virt_z {
    double x;
    double y;
    struct z_wrap {
        virt_z &parent;

        operator double() { return parent.x * parent.y; }
        z_wrap &operator=( double in ) {
            parent.x = in;
            parent.y = 1;
            return *this;
        }
        z_wrap( virt_z &in ) : parent( in ) {}
    } z;

    virt_z() : z( *this ) {}
    virt_z( virt_z const &o ) : x( o.x ), y( o.y ), z( *this ) {}
};

https://ideone.com/qqgj3D

Authority

And what about your poor client that has to read this interface and figure out that to set z they need to assign to z through an overloaded operator= of an inner wrapper class. The simple getter and setter function style of the OP MyClass1 is better than both MyClass2 and your solution because it is clear from reading the interface how to use it. Given that all three compile to the same code, I contend that MyClass1 should be favored. – Antonioantonius

@user1131467 Different things fit different situations. As for MyClass1, any function that does more than "set z" should not be called "SetZ()", and that's the fundamental problem with the getter/setter formalism. If setting something has side effects, then you're not really just setting some state. Likewise, you can seldom really just change an interface to add side effects within the same object like that. It's just a myth perpetuated by OO textbooks. – Authority

Why should the user care about implementation details of the interface? They should just use it naturally. – Kekkonen

Answer 3

It's not dangerous as such, it is just more difficult to use (for the class user). MyClass2 obfuscates and clutters the interface.

It might make it less tedious for you to write the code but you only have to write the code once - the interface will be reused many times. Therefore the code reader should be favored.

MyClass1 is superior to both MyClass2 and Potatoswatter solution for this reason.

(Even if there was a way in C++ to do this:

class MyClass3
{
    double x, y, z;
}

MyClass3::z::operator=(double) { ... } // property set
double MyClass3::z::operator() { ... } // property get

like in C# it would still be a bad idea I think because this sort of "hidden" code leads to bad assumptions that can slow down debugging. The function style at least lets you think there might be something more complicated going on than a primitive copy.)

Antonioantonius