Any problems with this C++ const reference accessor interface idiom?

Question

I was converting a struct to a class so I could enforce a setter interface for my variables.
I did not want to change all of the instances where the variable was read, though. So I converted this:

struct foo_t {
    int x;
    float y;
};

to this:

class foo_t {
    int _x;
    float _y;
public:
    foot_t() : x(_x), y(_y) {  set(0, 0.0);  }

    const int &x;
    const float &y;

    set(int x, float y)  {  _x = x;  _y = y;  }
};

I'm interested in this because it seems to model C#'s idea of public read-only properties.
Compiles fine, and I haven't seen any problems yet.

Besides the boilerplate of associating the const references in the constructor, what are the downsides to this method?
Any strange aliasing issues?
Why haven't I seen this idiom before?

Answer 1

Your approach is not flexible. When you have a getter / setter for every variable it means you don't have to rewrite your set method if you add something to your class.

It's not good because you can't have const and non-const getters (which are used rarely, but sometimes could be useful).

You can't copy references, therefore, your class becomes non-copyable.

Also, having referenced initialized in your class means extra memory and if we're talking about, for example, vertex class (though I think it shouldn't be a class actually), this could become a disaster.

---

[Everything that follows is completely subjective]

Purpose of getters and setters, to my mind, is not about simple modification, but rather about encapsulating a sequence of actions that results in value modification or returns the value (which we treat as visible result).

Personally struct in case of your example would be more effective, because it wraps POD data and logically "structurizes" it.

Answer 2

There is an aliasing issue in that because you expose a reference to the foo_t's internal data, it's possible for code external to a foo_t object to hold on to references into its data beyond the object's lifetime. Consider:

foo_t* f = new foo_t();
const int& x2 = f->x;
delete f;
std::cout << x2; // Undefined behavior; x2 refers into a foo_t object that was deleted

Or, even simpler:

const int& x2 = foo_t().x;
std::cout << x2; // Undefined behvior; x2 refers into a foo_t object that no longer exists

These aren't particularly realistic examples, but this is a potential issue whenever an object exposes or returns a reference to its data (public or private). Of course, it's just as possible to hold on to a reference to the foo_t object itself beyond its lifetime, but that might be harder to miss or to do by accident.

Not that this is an argument against what you're doing. In fact I've used this pattern before (for a different reason) and I don't think there's anything inherently wrong with it, aside from the lack of encapsulation, which you seem to recognize. The above issue is just something to be aware of.

Answer 3

One problem is that your class is no longer copyable or assignable, and so can't be stored in C++ containers like vectors. Another is that experienced C++ programmers maintaining your code will look at it and exclaim "WTF!!" very loudly, which is never a good thing.

Answer 4

You could also do something like this, which works for built in types: (Sorry if this code snippet contains errors, but you get the idea)

template <typename T, typename F>
class read_only{
   typedef read_only<T, F> my_type;
   friend F;

public:
   operator T() const {return mVal;}

private:
   my_type operator=(const T& val) {mVal = val; return *this;}
   T mVal;
};


class MyClass {
public:
   read_only <int, MyClass> mInt;
   void MyFunc() {
      mInt = 7; //Works
   }
};

AnyFunction(){
   MyClass myClass;
   int x = myClass.mVal; // Works (okay it hasnt been initalized yet so you might get a warning =)
   myClass.mVal = 7; // Error
}