Cppcoreguidelines: C.131 Avoid trivial getters and setters

[Jon12345]

One reason people write trivial getters and setters is to allow some future modifications without changing client code.
The note in this item says, "A getter or a setter that converts from an internal type to an interface type is not trivial (it provides a form of information hiding)."
A future change might be to change an internal type to a calculated type. Or to log every time the setter is called.

I also see this proposal http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3888.pdf (Herb Sutter is an author), includes getters and setters such as:
void set_tolerance(double tolerance);
double get_tolerance();
I can't see the implementation so I don't know if that's an internal type. But that's the point - as a client of this code, I shouldn't be exposed to whether it's an internal type or not.

[nadiasvertex]

I see a lot of justification for verbose code around "someday we might want to...". Trivial getters and setters complicate interfaces unnecessarily.

The paper you reference canonicalizes a pre-existing interface. However, and more importantly, the interface is a driver that involves access to unknown backends. The "setters" don't qualify as trivial because, for example, "set_tolerance()" may delegate the operation to an unknown, opaque entity.

A trivial getter/setter is when you have something like:

class data {
int stuff;
public:
  int get_stuff() { return stuff; }
  void set_stuff(int _stuff) { stuff=_stuff; }
}

The functions don't add any value. Clarity and simplicity is here sacrificed on the altar of "what if, someday..."

[JiriDockal]

What about debugging? You can't place a breakpoint on simple member. Yes, I know about memory breakpoints but working with them is far from ideal.

[nadiasvertex]

IMHO, it is only occasionally useful to put a breakpoint on a _trivial_
getter or setter. I would say that falls into the bucket of "one day
maybe...", which again IMHO is not a good reason for engaging in a practice
that complicates and obfuscates simple code.

On Mon, Jan 25, 2016 at 3:48 AM JiriDockal [email protected] wrote:

What about debugging? You can't place a breakpoint on simple member. Yes,
I know about memory breakpoints but working with them is far from ideal.

—
Reply to this email directly or view it on GitHub
https://github.com/isocpp/CppCoreGuidelines/issues/507#issuecomment-174439384
.

[LegalizeAdulthood]

IMO, this guideline is completely misnamed. The example demonstrates why: it isn't about trivial getters and setters, but about PODs disguised as a class.

If the class contains only trivial getters and setters, then I agree with the guideline. Otherwise, the class is encapsulating some behavior and the data members represent its internal state and I don't want client code coupled directly to my internal state. This is the entire reason that many guidelines recommend that getters and setters be used in the first place.

[jepessen]

I want to use trivial getters/setters because in this way the class has an uniform interface if trivial ones are mixed with complex ones. In addition to this, I think that trivial getters and setters add only few character and in order to maintain code readability this is the last think that we should care about (I don't think that code readability is affected by this).

Inviato da Outlook Mobile

On Mon, Jan 25, 2016 at 9:42 AM -0800, "Richard Thomson" [email protected] wrote:

IMO, this guideline is completely misnamed. The example demonstrates why: it isn't about trivial getters and setters, but about PODs disguised as a class.

If the class contains only trivial getters and setters, then I agree with the guideline. Otherwise, the class is encapsulating some behavior and the data members represent its internal state and I don't want client code coupled directly to my internal state. This is the entire reason that many guidelines recommend that getters and setters be used in the first place.

—
Reply to this email directly or view it on GitHub.

[nadiasvertex]

"A foolish consistency is the hobgoblin of little minds, adored by little statesmen and philosophers and divines." - rwe

I mean no offense by that, but I do think it is worth reflection when the argument is consistency.

Interfaces should satisfy the principle of least surprise, not the principle of "everything should be the same." When accessing a value is a trivial operation, it should be referenced directly. When it is not, it should be referenced indirectly.

This has many virtues, not the least of which is helping your users to understand what may be fast and what may be slow. Also, there is no inconsistency when the pattern is "simple values are data, and calculated values are functions." You can even have consistent naming to make your interface more guessable.

That is, after all, why we have both data members and function members.

[Sqeaky]

Pithy quotes do not really advance the conversation. We need real reasons for making decisions.

The "someday maybe" comes soon if you want to pass things with getters and setters to template functions. If everything has getters for the data members then it is easy to make the template simple even if it must accept classes with trivial and non-trivial getters.

Also, I have never been burned by having trivial getter/setter, but I have been burned by failing to add them to a class. I personally would not write an API with exposed data members unless I am exposing everything on that struct and the user is intended to understand the binary layout.

[nadiasvertex]

pith·y
ˈadjective

1. (of language or style) concise and forcefully expressive.

Not every famous quote is useful, but some capture important principles in a concise way. Principles are far more important than laws. When people understand principles, they don't need laws. What laws exist should aid people in understanding the underlying principles.

Not everything is an API. In fact, the vast majority of code that anyone writes is not an API. Note too that this item is not saying "never use getters and setters." The key is _trivial_. When you have a data structure which is private to your application or library, and when you are just trying to retrieve values stored, or store a simple value, writing a function is unnecessary and distracting.

Think of getters and setters as insurance. You don't buy more insurance than you need. You buy insurance for expensive things, where failure would be catastrophic. Your health. Your house. Your car. Heirloom jewelry. Maybe your phone. You own many things which might break, but it happens rarely, and they generally exceed their useful lifetime and are replaced by something completely different. You could insure everything, but it has a cost.

You can wrap getters and setters around everything, but it has a cost. There should be a _reason_ for making them, and the reason should be good. I don't personally consider "maybe some day" a good reason for trivial getters and setters to exist. Nor do I consider "this one time I didn't do it and it hurt" a good reason. I would consider "every single time I don't do this it hurts" a good reason. Or even "most of the time when I don't do this it hurts" a good reason.

However, in my experience, most of the time omitting trivial getters and setters is not painful, results in less scaffolding, and provides important signals to the user.

[Sqeaky]

I do not think word games contribute to the conversation either.

One example of a word game is dismissing debugging as "one day maybe", seeing the return values of functions can be valuable as not all debuggers show all values all the time. Most C++ developers will spend far more time with a debugger open than writing code, even counting time added by creating getters/setters.

I am not aware of costs to getters/setters. Compilers optimize them out. IDEs provide refactoring tools to write them. Developing them manually takes seconds. So they have no real machine or human cost. Please provide examples of costs.

Notable people such as Herb Sutter (in the first post) have concrete examples using that seem reasonable. Others have provided reasons and examples and there has been no meaningful counter-reasoning against any the arguments here.

Trivial getters/setters provide guarantees on encapsulation which are important in Object Oriented code, because we cannot know now that they will always be trivial. Once these decisions are enshrined in an APIs they are stuck and exposing data members can have a huge development cost. One concrete example where trivial getters/setters would have saved many man hours would be in the development of the Java access libraries that came before JNI. 2 different companies had 2 different teams of developers that exposed C struct members in APIs, now those APIs are functionally useless because they have been eclipsed by the current JNI interface where everything is wrapped in function calls: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/intro.html#historical_background

In libraries providing API getters/setters are critically important with regards to cost management, though I would concede that trivial getters/setter may not matter in application code or on PODs intended for passing data. Though any time I see public data members I see it as a code smell, and I expect to see bugs or issues associated with them.

[nadiasvertex]

I really think you are missing the point. TRIVIAL getters and setters. That
is what we are talking about. I am also not talking about public interfaces
to major APIs. I have said that at least twice now.

I am not dismissing debugging in general as "one day maybe." I am saying
that the probability that you are going to need to catch a read or write to
a particular _trivial_ data member everywhere it occurs is incredibly
small. That is absolutely "one day maybe."

class c {
int a;
public:
int get_a() { return a; }
void set_a(int _a) { a= _a; }
}

is drastically different than:

struct c {
int a;
}

There is a _lot_ of scaffolding there that generally serves no purpose, and
interferes with someone reading the code. I know that the compiler can
optimize those calls out most of the time. But writing the code:

void f(c &o) {
auto v = o.get_a();
}

compared to:

void f(c &o) {
auto v = o.a;
}

Is needlessly opaque.

What does get_a() do? Is it "fast" or "slow"? Does it have side effects? In
order to communicate that information you now have to write documentation.
Whereas simply writing "v = o.a", the programmer knows that the access is
fast, and the programmer knows that there are no side-effects. This is
crucial information in writing reliable, robust code. It also _aids_
debugging because you can be certain there are no side-effects from a
simple variable read or write.

As we all know, objects provide encapsulation. That is great when the state
is complex and the user should be protected from it. However, that is
horrible when the state is simple and the user _must_ be privy to it. That
is the entire premise of trivial getters and setters: the user needs this
information, and you are not doing anything to mediate the experience for
the user. In this situation the opaque veil of encapsulation does more harm
than good.

In this situation, the argument that "someday I may need to do X or Y and
therefore I have to wrap all these values" is not useful. You are, in
effect, claiming that the language feature which allows public data members
in classes and structs is a misfeature and should be removed. I feel that
is, at best, a tenuous position.

As for how easy IDEs make it to write the code above, that is debatable.
C++ is nowhere near the level of Java and C# where automation is concerned.
However, even if it were, the important point is not how easy code is to
_write_, but how easy it is to _read_. Code will be read many, many more
times than it is written. I would argue that "v = c.a" and "c.a = v" is
much simpler to understand than "v = c.get_a()" or "c.set_a(v)". Simply
because you have no idea what may happen in those functions.

There are other arguments about functions providing an implicit contract.
If you change what a getter or setter does, you have implicitly changed the
contract. That means that writers of getter/setter functions and _users_ of
getter/setter functions have an increased communication burden in order to
set expectations.

For these reasons and others, I feel that trivial getters and setters
should be avoided.

On Tue, Jan 26, 2016 at 8:37 PM Sqeaky [email protected] wrote:

I do not think word games contribute to the conversation either.

One example of a word game is dismissing debugging as "one day maybe",
seeing the return values of functions can be valuable as not all debuggers
show all values all the time. Most C++ developers will spend far more time
with a debugger open than writing code, even counting time added by
creating getters/setters.

I am not aware of costs to getters/setters. Compilers optimize them out.
IDEs provide refactoring tools to write them. Developing them manually
takes seconds. So they have no real machine or human cost. Please provide
examples of costs.

Notable people such as Herb Sutter (in the first post) have concrete
examples using that seem reasonable. Others have provided reasons and
examples and there has been no meaningful counter-reasoning against any the
arguments here.

Trivial getters/setters provide guarantees on encapsulation which are
important in Object Oriented code, because we cannot know now that they
will always be trivial. Once these decisions are enshrined in an APIs they
are stuck and exposing data members can have a huge development cost. One
concrete example where trivial getters/setters would have saved many man
hours would be in the development of the Java access libraries that came
before JNI. 2 different companies had 2 different teams of developers that
exposed C struct members in APIs, now those APIs are functionally useless
because they have been eclipsed by the current JNI interface where
everything is wrapped in function calls:
https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/intro.html#historical_background

In libraries providing API getters/setters are critically important with
regards to cost management, though I would concede that trivial
getters/setter may not matter in application code or on PODs intended for
passing data. Though any time I see public data members I see it as a code
smell, and I expect to see bugs or issues associated with them.

—
Reply to this email directly or view it on GitHub
https://github.com/isocpp/CppCoreGuidelines/issues/507#issuecomment-175333500
.

[LegalizeAdulthood]

@nadiasvertex Your example points out exactly what I was saying earlier. This guideline is misnamed. The guideline is more appropriately named something like "prefer simple structs for aggregate data with no behavior.".

Do you agree that human behavior is such that people will simply read the guideline title and think they know everything the guideline is talking about? All this subtlety of a "basically a POD" style "class" vs. an API object where data is more properly encapsulated is lost when the title just screams "THERE IS A PERMANENT THOUGHTLESS BAN ON TRIVIAL GETTERS AND SETTERS."

[nadiasvertex]

Yes, I agree. I think you make a very good point.

On Wed, Jan 27, 2016 at 4:59 PM Richard Thomson [email protected]
wrote:

@nadiasvertex https://github.com/nadiasvertex Your example points out
exactly what I was saying earlier. This guideline is misnamed. The
guideline is more appropriate named something like "prefer simple structs
for aggregate data with no behavior.".

Do you agree that human behavior is such that people will simply read the
guideline title and think they know everything the guideline is talking
about? All this subtlety of a "basically a POD" style "class" vs. an API
object where data is more properly encapsulated is lost when the title just
screams "THERE IS A PERMANENT THOUGHTLESS BAN ON TRIVIAL GETTERS AND
SETTERS."

—
Reply to this email directly or view it on GitHub
https://github.com/isocpp/CppCoreGuidelines/issues/507#issuecomment-175881471
.

[Sqeaky]

@LegalizeAdulthood That is a brilliant perspective on this.

This should probably be multiple rules. At least one describing what API designers might want to do to preserve inheritance and encapsulation, some describing what application devs might want to do and some describing when you want to make a POD for use as a parameter.

[BjarneStroustrup]

What is it we really want? It seems to me that this discussion is not just about _trivial_ getters and setters. Are we looking for an ABI? If so, getters and setters are not by themselves sufficient; they would have not be non-inline. Is what we want some sort of discussion or API design? Which features have which implications when used in an interface? Could someone outline/draft something that they'd like to see? It would probable be a major addition.

[LegalizeAdulthood]

@BjarneStroustrup The number of questions you raise leads me to think that this guideline isn't following Single Responsibility Principle, LOL. It seems to have been written with one use case in mind, but was stated as a general proposition beyond the specific use case. Maybe all we need to do is explicitly state the use case for which this guideline was intended and somehow fold the use case into the guideline title?

[nadiasvertex]

@BjarneStroustrup I think the guideline is good, except that it may need to be renamed. Something like "Prefer structs with minimal (or no) functions for data transport."

With respect to getter/setter behavior in other situations, I'm guessing you mean what kind of guidance do we want, not what should getters and setters look like ideally (ie. syntax).

I would like to see something like...

• How much encapsulation is enough / too much?
  
  
  
  • APIs
  
  
  • What are the costs and benefits?
    
    
    
    
    
    • Maintenance
    
    
    
    • Documentation
    
    
    
    • Implicit contracts
    
    
    
    • Lack or presence of side effects
    
    
    
    • Not a moving target
    
    
    
  
  
  • "Public" facing APIs with binary compatibility guarantees
  
  
  • Effects of in-lining
  
  
  • Other interactions that would break binary compatibility
  
  
  • "Driver" models, versus direct-implementation
  
  
  • "Internal" facing APIs with source compatibility
  
  
  • Balancing the costs of scaffolding with the benefits of interception
  
  
  • "Just code": classes which are not expected to be exposed outside of some library
  
  
  • How to determine when something is in this category.
  
  
  • How to decide when something should be promoted into an "API"
  
  

I also think that designing a good API is not a well understood art. Although it might be a topic for an entire book, I would look to see solid guidance on how to design "good" APIs. What are some good principles? (Guess-ability, least-surprise, orthogonality?)

[Jon12345]

Note that there's already this related guideline: "C.2: Use class if the class has an invariant; use struct if the data members can vary independently".

[borissmidt]

To be fair a trivial getter and setter is when you just return a pointer to private member when you do a get function and does a copy on a set function. Most of the time the code then shares this pointer to another class and this violates encapsulation completely.
i.e. if you borrow your friends car you should not borrow it to another friend.

So if an other class gets the pointer and edits it while the setter is only used at an init or a constructor. then this is a trivial getter and a useless setter.

It makes the code much cleaner if you make that member public and this makes it clear that you are not working with a copy. But if a getter returns a copy or somekind of warped class to limit the aces to its members then it might be useful.

So a non trivial getter is one that ether returns a copy, returns a wrapped class, or returns a const ref/ptr. a non trivial setter is one that does a deep copy or does some range or type checking.

I'm not allowed to use public variables nor protected at my work, but most of the time a pointer is return with a getter...

[AndrewPardoe]

Bjarne, you've outlined what you want to write on Feb 1, 2016 (above.) Assigning to you to complete.

[HenryMiller1]

To summarize,

Notes

Before applying this rule check if Rule Rc-struct applies.

Next, consider if you should have the getter/setter at all. There is a purist argument in object oriented programming that classes should do things not provide data. While C++ is not a pure object oriented language purist argument is worth considering.

The key to this rule is whether the semantics of the getter/setter are trivial. A getter/setter is trivial if the following conditions apply

• The implementation of the getter is a simple return of value
• The implementation of the setter is a simple assignment
• If either of the above fail to apply in the future you would not be able to use a tool to change to using getter/setter functions.

Not being able to use a tool this means that you have provide binary compatibility guarantees, along with PIMPL implementing trivial getters/setters in a .cpp is good "just in case" protection. Sometimes it means that are political issues in your organization around change. This should not be used an excuse to not have a tool available to do these refactors as needed.

Enforcement
Flag multiple get and set member functions that simply access a member without additional semantics. Make sure violators know how to use refactoring tools to add these if they need non-trivial implementation latter.

[hsutter]

Editors call summary: This has been good discussion and feedback but this is a controversial area where opinions may vary and what we have is probably the best recommendation we can have for now. No change at this time.