3
\$\begingroup\$

I have made a facility to perform operator= dispatching to a member setter function. The purpose is to enable accessing members (like using object.member = value;) while at the same time respecting encapsulation.

My implementation looks like this:

#include <iostream>

#define OPEQ_SETTER_GETTER( OwnerType, Type, Function, Name ) \
    accessor< Type, OwnerType, decltype(&OwnerType::Function) > Name = \
    accessor< Type, OwnerType, decltype(&OwnerType::Function) >(this, &OwnerType::Function); \
    friend class accessor< Type, OwnerType, decltype(&OwnerType::Function) >;

// A generic accesor class
template <typename T, class Owner, class Setter> struct accessor {
    accessor ( Owner* o, Setter &&f ): owner( o ), setter( f ) { }
    accessor &operator=( const T &value ) { setter( owner, data, value ); return *this; }
    operator T() const { return data; }
private:
    Owner *owner;
    Setter setter;
    T      data;
};

// An example of applying the idiom
class example
{
    private:
    static void setx( example *this_handle, double &data, const double &value )
    {
        std::cout << "Setting data to: " << value << '\n';
        this_handle->another_member = 85656;
        std::cout << "another_member is: " << this_handle->another_member << '\n';
        data = value;
    }

    std::size_t another_member = 0;

    public:
    example( )  {}

    OPEQ_SETTER_GETTER( example, double, setx, x );
    // Will expand to:
    //accessor< double, example, decltype(&example::setx) > x = 
    //      accessor< double, example, decltype(&example::setx) >(this, &example::setx);
    //friend class accessor< double, example, decltype(&example::setx) >;

};

int main()
{
    example e;
    e.x = 2*3.14159;
    std::cout << "New value: " << e.x << '\n';

    return 0;
}

Demo

The questions I have questions are:

  1. Does anybody see any issues with the above?
  2. Does anybody have any suggestions in implementing this without using the preprocessor and still keep the interface similarly compact?
\$\endgroup\$
3
\$\begingroup\$

Design

I think it obfuscates the code with no real advantage.

// This adds an object `x` to the class `example` that
// when assigned a double will forward the
// value to the private method `setx()`
OPEQ_SETTER_GETTER( example, double, setx, x );

I think it would be easier to add a public function that forwards the value.

public:
void publicSetX(double v) {setx(v);}  // ? easier to read.

Also I add my usual rant about getters/setters breaking encapsulation by exposing internal implementation details.

Code Review

Make sure this is noted as a non-owning pointer.

Owner *owner;

When you construct the object. Owner should never be nullptr

accessor ( Owner* o, Setter &&f ): owner( o ), setter( f ) { }

So pass owner by reference to indicate this. Its OK to store as a pointer (as its hard to copy objects with references without additional work). But your interface should be as clean as possible so there is no misunderstanding. Pass by reference to indicate it can't be nullptr and that this object is NOT taking ownership.

The setter value f can be bound to an r-value reference. But the parameter f will not pass it on as an r-value reference. Named objects can not be r-value references. So you need to use std::forward to activate perfect forwarding. Otherwise you are just activating the copy constructor (not the move constructor).

accessor ( Owner& o, Setter &&f ): owner( &o ), setter( std::forward<Setter>(f) ) { }

OK. For doubles passing a reference around is fine. But this is supposed to be generic. You also need to allow for move assignment.

// What if T is big.
// I would want to move the `value` into the object using assignment.
accessor &operator=( const T &value ) { setter( owner, data, value ); return *this; }

Reading the value generates a copy. This is not usually desirable. Especially if T is large.

operator T() const { return data; }

I would do:

operator T const&() const { return data; }
\$\endgroup\$
  • \$\begingroup\$ @Loki_Astari Amazing answer! Can you please clarify why this implementation would break encapsulation? accessor is a utility class only meant to pass the functions and objects around and the setx function and members are not exposed outside the example class. \$\endgroup\$ – Steve Rodeen Aug 28 '17 at 15:57
  • \$\begingroup\$ @SteveRodeen That comment on setters/getters was more a general comment that they break encapsulation by exposing the internal types used. By allowing the state to be extracted modified and put back you are locking your class into a specific implementation (because you have leaked type information through the interface). I prefer never to expose members to read/modification (even if it is through a function). Then I never expose internal type information thus allowing me to change the implementation without changing the public interface. Objects are manipulated by their methods which are verbs \$\endgroup\$ – Martin York Aug 28 '17 at 18:51
  • \$\begingroup\$ Car c;c.setSpeed(c.getSpeed() + 10); Bad interface exposes that I store the speed of a car as a number. Must support that forever. Car c;c.accelerate(10); Internal representation of speed can be anything I like. \$\endgroup\$ – Martin York Aug 28 '17 at 18:53
  • \$\begingroup\$ @Loki_Astari Oh I see. But in the second example how would you enable the user to set the speed to an absolute value or read the current speed? \$\endgroup\$ – Steve Rodeen Aug 28 '17 at 19:00
  • \$\begingroup\$ The question becomes would I need to let them set it to an absolute speed? You should never need to read the speed. Now display the speed that is a different question. You have an appropriate interface that allows the car to display the speed. Car c;c.display(dashboard); \$\endgroup\$ – Martin York Aug 28 '17 at 19:05
2
\$\begingroup\$

I won't repeat any of the excellent points @Loki Astari already mentioned.

Asymmetric Interface

The current implementation only allows for custom setters, but not for custom getters. This might be fine for trivial implementations of those, but will hinder reusability in more demanding cases.

Unnecessary encapsulation breakage

There's no need for a friend class accessor<...> definition in the macro.

Unnecessary convention introduction

The implementation requires that each setter function is static and a class method. Both requirements might be too restricting for all use cases.

Naming

I get it, naming is hard.

accessor might be borderline acceptable, but property (as those from C#) or member might be more fitting.

Also, just looking at the name, what should OPEQ_SETTER_GETTER do? Get a "setter"? Declare a "setter/getter" object? OPEQ_DECLARE_PROPERTY would be a far better fit IMHO, better conveying its intended purpose.

User defined conversion problems

If T has its own user defined conversion operator, you might get into problems when "chaining" conversions. Example:

class TestA {
public:
    operator int() const { return 10; }
};

class TestB {
    static void setTestA(TestB *this_handle, TestA& data, const TestA& newValue) {
        data = newValue;
    }
public:
    OPEQ_SETTER_GETTER( TestB, TestA, setTestA, a );
};

int main() {
    TestB b;
    std::cout << "b.a: " << b.a << "\n";
}

If b.a were truly of type TestA, it would be automatically converted by the compiler to int, thus calling the correct overload operator<<(std::ostream&, int). However, since b.a is of type accessor<TestA, TestB, decltype(&TestB::setTestA)>, the compiler does not find that conversion (it stops looking after the conversion to TestA). Make sure to document this (sometimes surprising!) behavior appropriately!

Thinking ahead...

If you allow custom getters as well, you might find cases where both getter and setter only refer to another variable (other than accessor::data). It might be worth to consider making a special case for those.

If I had to implement something like that, I would probably start with something like this:

template<typename PropertyType>
class property {
    std::function<void(PropertyType&, const PropertyType&)> setter;
    std::function<PropertyType(const PropertyType&)> getter;
    PropertyType data;

    // implementation here
};

template<typename PropertyType>
class property_alias {
    std::function<void(const PropertyType&)> setter;
    std::function<PropertyType()> getter;

    // implementation here
};

// usage
class temperature {
    double fahrenheit_to_kelvin(double fahrenheit) { ... }
    double kelvin_to_fahrenheit(double kelvin) { ... }
public:
    property<double> kelvin {
        [&](auto& data, const auto& value) {
            if(value < 0) throw std::exception("temperature too low!");
            data = value;
        },
        [](auto& data) {
            return data;
        }
    };
    property_alias<double> fahrenheit {
        [&](const auto& value) {
            kelvin = fahrenheit_to_kelvin(value);
        },
        [&]() {
            return kelvin_to_fahrenheit(kelvin);
        }
    };
};
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.