Why do people write private-field getters returning a non-const reference?

Question

We can all agree on public variables being bad for encapsulation and all that. However, I noticed a lot of code that does this type of stuff:

class foo {
private:
    int integer_;
    string someString_;
    // other variables
public:
    int& integer() { return integer_; }
    string& someString() { return someString_; }
    // other "functions"
}

int main() {
    foo f;
    f.integer() = 10;
    f.someString() = "something";
    return 0;
}

I have seen this being used in many places and I don't get why. Basically it returns a reference to the data and thus exposes it directly to the outside. So encapsulation is not really achieved, not from any perspective.

Why is this commonly used?

Testerman

"Why is this commonly used?" Because of common stupidity. – Agnosia

It's commonly used because many programmers don't really know what they are doing. – Perpetua

So they basically think they are solving a problem while all they are doing is just adding verbosity into their code? – Testerman

It's bad as other commenters already said. Look here – Celt

Unfortunately common practice and best practice are not always the same. Btw I believe this is not common... – Aten

could you give some reference? I don't recall ever seeing such a code :) – Steelworks

@Testerman The average programmer drinks too much coffee ;-) – Agnosia

"So they basically think they are solving a problem while all they are doing is just adding verbosity into their code?" - perfect description of getters and setters. – Sfumato

Related: Does it make sense to provide non-const reference getter – Honorary

Related: Breaking encapsulation by returning non-const references to members – Honorary

Here is a Regex for Visual Studio which may help find cases of methods returning non-const references: (?<!const\s+(?:\w+\s+)?|::)\b\w+(?:::\w+)*\s*(?:<[^<>]*>\s*)*&\s+\w+(?:::\w+)*\s*\( — May be not perfect, someone can suggest improvements! – Honorary

Answer 1

There's a recurring mantra, that getter/setter functions should be used to encapsulate your data. Hence many (unexperienced or coffee-overloaded) programmers get the idea they should use something like:

int& integer() { return integer_; }

but that isn't much different from simply writing:

class foo {
public: // <<<
    int integer_;
    string someString_;
    // ...
};

Well, it adds a function call, but you cannot control what the client does with the reference.

---

If you really want to provide a getter function write:

const int& integer() const { return integer_; }

A corresponding setter function looks like:

void integer(const int& value) {
    integer_ = value;
}

Detribalize

Yeah it just adds verbosity to the code that is unnecessary. Yet many use it. It just blows my mind :| – Testerman

Yeah, this is just like size() from std::vector – Testerman

Why would you not write: int integer() const { return integer_; }? – Perpetua

@NeilButterworth The difference is minor and my example extrapolates well for other types than int (besides keeping tight to OP's example). – Agnosia

@NeilButterworth Why not? you are not endangering the private variable for any changes. You are passing it as const – Testerman

@Testerman It's more typing, more code to read, and it is probably less efficient. If you look at similar functions in the Standard Library, they return integer values. – Perpetua

Just wanted to draw your attention to my own answer below, since I'm arguing with yours somewhat. – Pitsaw

@πάνταῥεῖ So the code you just provided fairly looks like an explicit version of C#'s properties. – Testerman

@ein We're always arguing somehow. I don't know what your point actually is, that it would conflict with my answer. – Agnosia

@Testerman Yes, that's the idiomatic way doing properties in c++. – Agnosia

@πάνταῥεῖ: It conflicts with the sentence "that isn't much different than writing etc. etc." - my answer claims that it is different. – Pitsaw

Having a public getter and setter is no different to having a public member, other than increased verbosity – Sfumato

Answer 2

I have to partially disagree both with @πάνταῥεῖ and @Rakete1111 's answers, considering how a class's definition is something that may evolve over time.

While it's true that, often, these getter methods are written by someone who's just heard the "no exposing members" mantra, they can also have legitimate uses:

1. The getter method may later be modified to include some kind of validity check or resource allocation code before returning the reference - which direct access to the data member does not allow. While this means changing the class's code, it does not require changing the class user code. Also, if the implementation of the getter is not exposed in the class header, it might not even require recompiling the class user code. Note: Doing so is probably a sign of some other poor design choice.
2. The getter method may be overridden by a subclass (in which case it is often made a virtual method) in a similar way to the above.
3. The getter method may later replace its return type with a proxy for the original data member type, rather than a reference to an actual member - which may no longer even exist. Think of how vector<bool> works; when you call its operator[] you don't get a boolean&, you get some kind of proxy which, when assigned or assigned-to, does the appropriate bit extraction or setting.
4. A non-const getter is not usable for non-const instances. So actually it does limit access relative to exposing the member outright. Whether the author of OP's example class actually intended this is a different question...

To sum up: The "dummy" non-const-reference getter can be a stub for other, meaningful, code.

That being said, it is often a good idea to just make the getter return a const reference or a value. Or just exposing the field in those cases where it's appropriate (and there are some of those too).

Pitsaw

It really makes some sense what you are talking. I do realize C++ does not have C#'s properties which actually help controlling all these stuff easily and as you've said without the need to change user code. But again, comparing two different languages is not really valid – Testerman

Ever heard of YAGNI? – Perpetua

@NeilButterworth: (1) Yes, but - obligatory link for those who havent. (2) I may not going to need it, but who knows what the author of that code is going to need? – Pitsaw

(3) If you're designing an inheritance hierarchy, you may already need it (or at least - have use for it). (4) I've never actually written such getters myself :-) – Pitsaw

@ein "The getter method may later be modified to include some kind of validity check" Exactly that isn't possible exposing a non const reference! – Agnosia

@πάνταῥεῖ:: (1) Why not? {int& integer(){ if (life_is_good()) { return integer_; } else throw std::runtime_error("Life's not good!"); } (2) I suggested a few other uses in addition to that one. – Pitsaw

@ein Smells like poor design, but last resort to keep dangling access off. – Agnosia

@πάνταῥεῖ: (1) You're right, and I'll edit my answer to make that comment. (2) "No reason to do it" and "Doing it because of a questionable design choice" are two different things. (3) OP asked why are people writing these getters, and this is one of the possible reasons. – Pitsaw

Answer 3

I would strongly discouraged returning a non-const reference to a private variable. Not because it breaks encapsulation, but because it is unnecessary: Why not make the variable public in the first place?

Breaking encapsulation is bad, yes, but that does not mean that every variable should be private. Some variables are meant to be read and modified from the user, so it would make sense to make them public. For example, take std::pair, it has 2 public member variables, first and second. That's not bad practice.

The only time it doesn't make sense is when the variable is not supposed to be written to. That would be bad, as it would actually break encapsulation and make the whole program hard to debug.

Crowson

std::pair doesn't have any behaviour though - if your class has behaviour, then public data is fatal – Perpetua

@NeilButterworth Why? What's the difference? – Crowson

Exactly as @NeilButterworth said, pair has no behavior. It is just container for data. So this is the best way of doing stuff? Having containers different from actors? Other than the case of when you need friend – Testerman

Well, if you have a class with behaviour, such as std::string, what happens if you can gaily change the variables implementing the class? What happens with std::string if you directly change the pointer pointing to storage for the string? – Perpetua

@NeilButterworth The pointer is not meant to be modified, so it has to be private, yes. I never said that it makes sense for every variable, but sometimes, it does. Maybe you have a Window class where the width and the height are public, to be modified and read. Then, the next time the main loop reads the width and height, and modifies the window accordingly – Crowson

Why would you want the "main loop" to have to read these variables every time? In fact for a window class you not only want them to be private, but you want a SetSize( int x, int y ) function so that you don't need to do two updates of the GUI representation. – Perpetua

Hmm, @NeilButterworth std::string would not be the best example because it is a class with a lot of behavior. Yet you can still change characters from its internal character array, or change its internal variables from the outside – Testerman

@Testerman "or change its internal variables from the outside" but not directly, which is the point of all this. – Perpetua

@Crowson Conversely, if Window is more complex and needs to perform certain actions, such as managing its own buffer, you would want to disallow direct access to width and height, so you can force the user to call a member function that guarantees that all necessary actions will be done. That way, you can keep someone from accidentally increasing the window size without expanding the buffer, for example. – Tymbal

@NeilButterworth Yeah you are basically adding an operator that tells the class to access that particular char from the internal character array. Some people go really far when describing encapsulation and suggest that this is also not the best way, where they would send me to a point that i have no idea how are you supposed to do anything >.< – Testerman

@NeilButterworth Yeah, that would be better, you're right. But still, there may be some scenarios where a public variable is better, because someone has to have read and write access to it. – Crowson

Just wanted to draw your attention to my own answer below, since I'm arguing with yours somewhat. – Pitsaw

Isn't it missing non-const on I would strongly discouraged returning a non-const reference to a private variable? – Honorary

@Honorary Jup. Good catch! Thanks – Crowson

Answer 4

This construction may be used for debugging purposes.

If you have a public variable, you can't monitor its usage easily. Converting it to a pair of private variable and method-returning reference will allow you to put a breakpoint and/or log the calls.

However, separate getter and setter would serve the same purpose even better, so this is just an advantage over plain public variables.

Katz

Answer 5

I wrote this once. I planned later to go back and replace the field getter with something that returned a stack object to something that could be cast to the original type and assigned to by the original type. This allowed me to go back later and intercept all assignments to put validations in.

Kinda overpowered techinque. None of the other coders on the project could understand it at all. The whole stack got ripped out.

Chat