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.

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.

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

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.)