5

Lately I'm writing my getter and setters as (note: real classes do more things in getter/setter):

struct A {
  const int& value() const { return value_; } // getter
        int& value()       { return value_; } // getter/setter
 private:
  int value_;
};

which allows me to do the following:

auto a = A{2}; // non-const object a

// create copies by "default" (value always returns a ref!):
int b = a.value();  // b = 2, is a copy of value :)
auto c = a.value(); // c = 2, is a copy of value :)

// create references explicitly:
auto& d = a.value();  // d is a ref to a.value_ :)
decltype(a.value()) e = a.value(); // e is a ref to a.value_ :)
a.value() = 3; // sets a.value_ = 3 :)

cout << b << " " << c << " " << d << " " << e << endl; // 2 2 3 3

const auto ca = A{1};
const auto& f = ca.value();  // f is a const ref to ca.value_ :)
auto& g = ca.value(); // no compiler error! :(
// g = 4; // compiler error :)
decltype(ca.value()) h = ca.value(); // h is a const ref to ca.value_ :)
//ca.value() = 2; // compiler error! :)

cout << f << " " << g << " " << h << endl; // 1 1 1

This approach doesn't allow me to:

  • validate the input for the setter (which is a big BUT),
  • return by value in the const member function (because I want the compiler to catch assignment to const objects: ca.value() = 2). Update: see cluracan answer below.

However, I'm still using this a lot because

  • most of the time I don't need that,
  • this allows me to decouple the implementation details of my classes from their interface, which is just what I want.

Example:

struct A {
  const int& value(const std::size_t i) const { return values_[i]; }
        int& value(const std::size_t i)       { return values_[i]; }
  private:
   std::vector<int> values_; 
   // Storing the values in a vector/list/etc is an implementation detail.
   // - I can validate the index, but not the value :(
   // - I can change the type of values, without affecting clients :)
 }; 

Now to the questions:

  • Are there any other disadvantages of this approach that I'm failing to see?
  • Why do people prefer:
    • getter/setters methods with different names?
    • passing the value as a parameter? just for validating input or are there any other main reasons?
gnzlbg
  • 7,135
  • 5
  • 53
  • 106
  • 3
    One disadvantage is that it is more typing than making the data member public, and it doesn't give you much in return. – juanchopanza Aug 07 '13 at 15:46
  • 3
    I believe people generally pass in the value as a parameter for the exact reason that you said - input validation. As well as security, and really any other uses you could think of. You could make sure that an object has "permission" to change certain values, as well as validate to make sure that they aren't inputting invalid data. – Ricky Mutschlechner Aug 07 '13 at 15:46
  • 1
    "Simple C++ getter/setters" - if you want to stay same, why not `object.publicIvar = 42;`? –  Aug 07 '13 at 15:46
  • @H2CO3 separate implementation from interface. E.g. if my class has a container, and the getter/setter allows me to get a value, I don't have to change the client code if I change the type of the container (from vector to list, or whatever). – gnzlbg Aug 07 '13 at 15:48
  • I can't give a complete answer, but the reason setters usually get a value, is so that extra behavior and validation can be added to the setter without changing the classes behavior to the outside. Even when you don't need extra behavior at first, using this approach to a setter will allow you to add this in the future (or in a derived class!!). – JSQuareD Aug 07 '13 at 15:49
  • 5
    ...semantically, your get-setter has NO difference between it and a simple public member. If people want that behavior, they just make a public member, otherwise, we go with the more standard get set(value). – IdeaHat Aug 07 '13 at 15:50
  • @MadScienceDreams _It is_ semantically different. I can change the type of value without changing client code. It is not much, but it is something. I've added a second example to show that more clearly. – gnzlbg Aug 07 '13 at 15:56
  • this question is off topic for stackoverflow. – Daniel A. White Aug 07 '13 at 15:57
  • 4
    In production code, using `value() = 3` has a major disadvantage in that it requires more digesting to understand that value is overloaded as a setter and returns a reference that can be modified. When you are sustaining or writing new code using an existing API, you want readability to be easy and fast. `get()` and `set(value)` are explicitly so and completely self explanatory (i.e. No comments required). – Craig Aug 07 '13 at 16:07
  • 3
    This seems like a very messy way to do something that should be simple. If you need to "validate input" because your members might go out of sync with each other (e.g. a date struct, where "incrementing" the day may require the month and/or year to change) then you probably should be providing those operations (such as increment_day()), and not access to change the individual members. If the individual members don't have to sync up, but need to be in a particular range, you're probably better off turning those members into classes, and having them do their own validation. – Crowman Aug 07 '13 at 16:16

3 Answers3

6
  • Generally using accessors/mutators at all is a design smell that your class public interface is incomplete. Typically speaking you want a useful public interface that provides meaningful functionality rather than simply get/set (which is just one or two steps better than we were in C with structs and functions). Every time you want to write a mutator, and many times you want to write an accessor first just take a step back and ask yourself "do I *really* need this?".
  • Just idiom-wise people may not be prepared to expect such a function so it will increase a maintainer's time to grok your code.
  • The same-named methods are almost the same as the public member: just use a public member in that case. When the methods do two different things, name them two different things.
  • The "mutator" returning by non-const reference would allow for a wide variety of aliasing problems where someone stashes off an alias to the member, relying on it to exist later. By using a separate setter function you prevent people from aliasing to your private data.
Mark B
  • 95,107
  • 10
  • 109
  • 188
5

This approach doesn't allow me to:

  • return by value in the const member function (because I want the compiler to catch assignment to const objects ca.value() = 2).

I don't get what you mean. If you mean what I think you mean - you're going to be pleasantly surprised :) Just try to have the const member return by value and see if you can do ca.value()=2...

But my main question, if you want some kind of input validation, why not use a dedicated setter and a dedicated getter

struct A {
  int  value() const { return value_; } // getter
  void value(int v)  { value_=v; }      // setter
 private:
  int value_;
};

It will even reduce the amount typing! (by one '=') when you set. The only downside to this is that you can't pass the value by reference to a function that modifies it.

Regarding your second example after the edit, with the vector - using your getter/setter makes even more sense than your original example as you want to give access to the values (allow the user to change the values) but NOT to the vector (you don't want the user to be able to change the size of the vector).

So even though in the first example I really would recommend making the member public, in the second one it is clearly not an option, and using this form of getters / setters really is a good option if no input validation is needed.

Also, when I have classes like your second type (with the vector) I like giving access to the begin and end iterators. This allows more flexibility of using the data with standard tools (while still not allowing the user to change the vector size, and allowing easy change in container type)

Another bonus to this is that random access iterators have an operator[] (like pointers) so you can do

vector<int>::iterator       A::value_begin()     {return values_.begin();}
vector<int>::const_iterator A::value_begin()const{return values_.begin();}
...
a.value_begin()[252]=3;
int b=a.value_begin()[4];
vector<int> c(a.value_begin(),a.value_end())

(although it maybe ugly enough that you'd still want your getters/setters in addition to this)

rabensky
  • 2,864
  • 13
  • 18
  • pleasantly surprised indeed :) I started returning by value, but changed my mind along the way. Trying to reproduce now... – gnzlbg Aug 07 '13 at 16:08
  • Just notice that it's good for `int` and other basic types, but don't return by values larger classes like `string` or whatever. There will be an implicit copy you don't want if you return by value. – rabensky Aug 07 '13 at 16:10
0

REGARDING INPUT VALIDATION: In your example, the assignment happens in the calling code. If you want to validate user input, you need to pass the value to be validated into your struct object. This means you need to use member functions (methods). For example,

struct A {
  // getter
  int& getValue() const { return value_; }

  // setter
  void setValue(const int& value) {
    // validate value here
    value_ = value;
  }

  private:
    int value_;
};

By the way, .NET properties are implemented are methods under the hood.

Malganis
  • 338
  • 3
  • 5