85

Several questions about accessor methods in C++ have been asked on SO, but none was able satisfy my curiosity on the issue.

I try to avoid accessors whenever possible, because, like Stroustrup and other famous programmers, I consider a class with many of them a sign of bad OO. In C++, I can in most cases add more responsibility to a class or use the friend keyword to avoid them. Yet in some cases, you really need access to specific class members.

There are several possibilities:

1. Don't use accessors at all

We can just make the respective member variables public. This is a no-go in Java, but seems to be OK with the C++ community. However, I'm a bit worried about cases were an explicit copy or a read-only (const) reference to an object should be returned, is that exaggerated?

2. Use Java-style get/set methods

I'm not sure if it's from Java at all, but I mean this:

int getAmount(); // Returns the amount
void setAmount(int amount); // Sets the amount

3. Use objective C-style get/set methods

This is a bit weird, but apparently increasingly common:

int amount(); // Returns the amount
void amount(int amount); // Sets the amount

In order for that to work, you will have to find a different name for your member variable. Some people append an underscore, others prepend "m_". I don't like either.

Which style do you use and why?

einpoklum
  • 118,144
  • 57
  • 340
  • 684
Noarth
  • 3,981
  • 6
  • 23
  • 16
  • 5
    Small data-only structs aside, what gave you the impression public member variables would be ok in C++? Its usually a no-go as well. – Georg Fritzsche Sep 05 '10 at 19:36
  • 1
    Yes, it's a no-go in C++ as well. But if you worked with MFC you could get the assumption that it would be OK. – vobject Sep 05 '10 at 19:49
  • Most recently discussed as [set/get methods in C++ ](http://stackoverflow.com/questions/3632533/), which I found with the serach [*"c++ getter setter"*](http://stackoverflow.com/search?tab=newest&q=[c%2b%2b]%20getter%20setter). See also [Getter and setter, pointers or references and good syntax to use in c++? ](http://stackoverflow.com/questions/1596432/), [C++ getters/setters coding style](http://stackoverflow.com/questions/760777/) and probably others. – dmckee --- ex-moderator kitten Sep 05 '10 at 19:53
  • 4
    See [this article (PDF)](http://www.idinews.com/quasiClass.pdf) for why __getters/setters are just plain wrong__. – sbi Sep 05 '10 at 22:26
  • @Georg Fritzsche: I think I saw it in STL and boost, but I'm not sure actually. @dmckee: Yes, there are a bunch of posts on this topic and they are all interesting, but none answers my specific questions. – Noarth Sep 06 '10 at 04:25
  • I don't see how you question add anything much. It still just ask a stylistic question which was aldready asked multiples times here I'm pretty sure. Maybe you could point out where your question actually innovate? You don't even seem to point out that you might actually only need a get and not a set method or any other variant. – n1ckp Sep 06 '10 at 15:29
  • You'd have to force me to write public getters and setters in C++; they're plainly useless on their face unless they have a side effect. – hoodaticus Jun 01 '18 at 15:25
  • @sbi A zillion times I've seen people ranting "this-is-not-OOP", and I still struggle to figure out what OOP *is* then, when it comes to mutating an object's state =( The PDF doesn't shed any light on the matter, what do they all talk about, after all? – vines Jun 11 '20 at 18:13
  • 1
    @vines: In real OO, an object's interfaces (defined in a class in statically typed languages) shouldn't be concerned with the object's state, but only with the operations the object provides. That these operations alter the object's state is an implementation detail. – sbi Jul 11 '20 at 21:07
  • @sbi your link seems to be broken by now, but it sounds interesting, do you still have it? – Marses Aug 15 '23 at 10:40

10 Answers10

48

From my perspective as sitting with 4 million lines of C++ code (and that's just one project) from a maintenance perspective I would say:

  • It's ok to not use getters/setters if members are immutable (i.e. const) or simple with no dependencies (like a point class with members X and Y).

  • If member is private only it's also ok to skip getters/setters. I also count members of internal pimpl-classes as private if the .cpp unit is smallish.

  • If member is public or protected (protected is just as bad as public) and non-const, non-simple or has dependencies then use getters/setters.

As a maintenance guy my main reason for wanting to have getters/setters is because then I have a place to put break points / logging / something else.

I prefer the style of alternative 2. as that's more searchable (a key component in writing maintainable code).

Niall
  • 30,036
  • 10
  • 99
  • 142
  • 2
    Accepting a specific answer for community wiki questions is always a bit weird, because every answer helps. However, I think yours is the one best justified. I'm currently trying to avoid getter/setter methods wherever possible. It takes some time to come up with alternatives, but they are often better. If I need simple data as mentioned by you, I usually use a data-only struct. – Noarth Oct 08 '10 at 07:15
  • If you're having to debug accessors that's just another reason not to use them. Surely there are more interesting places to set a breakpoint. – hoodaticus Jun 01 '18 at 15:28
  • 1
    This is a great point to make that it is useful in terms of debugging, but I do agree there are probably alternative ways to obtain that information. Other then that my thoughts are GETS & SETS are to never be used together. Many suggest GET/SET allow you to modify it's return value, but then it's not a GET/SET it's a method performing some set of actions and should be named it's full intent on the process at hand. – Jeremy Trifilo Jan 09 '19 at 02:01
9

2) is the best IMO, because it makes your intentions clearest. set_amount(10) is more meaningful than amount(10), and as a nice side effect allows a member named amount.

Public variables is usually a bad idea, because there's no encapsulation. Suppose you need to update a cache or refresh a window when a variable is updated? Too bad if your variables are public. If you have a set method, you can add it there.

AshleysBrain
  • 22,335
  • 15
  • 88
  • 124
  • 2
    The canonical encapsulation means hiding of internal state and behavior details of a logical unit. If you do access a private data member via the public interface — then you *already* break encapsulation, regardless of the fact that you do it with accessors. Well encapsulated classes don't have accessors: instead they have active methods which use and influence internal state in a publicly-invisible ways. – ulidtko Mar 19 '12 at 04:41
  • 1
    Regarding the "changed requirements" scenario, e.g. mutating cache or updating UI, or sending a network request inside a `getId()` call: by stuffing that logic into a getter/setter you are violating the contract of the accessor method. You will not break the client code compilation in this way, but you certainly will break its runtime operation, which is even worse. Because nobody in a sane state of mind will assume that your getter could, say, do memory allocation sometimes, or your setter could write to a file system. In all these scenarios you need **redesign** for the new requirements. – ulidtko Mar 19 '12 at 04:52
  • 1
    #1 makes your intentions clearest, not #2. #2 pretends like it's encapsulating internal state by making a variable private, then undoes all of its work in hiding that state by allowing anyone, through the public accessors, to read and write that variable. #2 is not a case of clear intentions, it's just a shell game. – hoodaticus Jun 01 '18 at 15:22
7
  1. I never use this style. Because it can limit the future of your class design and explicit geters or setters are just as efficient with a good compilers.

    Of course, in reality inline explicit getters or setters create just as much underlying dependency on the class implementation. THey just reduce semantic dependency. You still have to recompile everything if you change them.

  2. This is my default style when I use accessor methods.

  3. This style seems too 'clever' to me. I do use it on rare occasions, but only in cases where I really want the accessor to feel as much as possible like a variable.

I do think there is a case for simple bags of variables with possibly a constructor to make sure they're all initialized to something sane. When I do this, I simply make it a struct and leave it all public.

Omnifarious
  • 54,333
  • 19
  • 131
  • 194
  • 4
    +1 for the "too clever". I would recommend not to abuse of the overload possibility, it just makes it harder on readers, and it's also harder to search the codebase. – Matthieu M. Sep 06 '10 at 06:49
  • -1 For "limiting the future of your class design etc.". You're designing the class as it is now. If you want to redesign it later, do so. What if that field goes away in the future? The accessor won't help you. And if you keep it, returning some junk value, you might having silent breakage you're not aware of. Don't over-design, otherwise you could just have a map of field-names-to-value-accessors and be done with it. – einpoklum Jan 21 '16 at 23:50
  • Why would you want "the accessor to feel as much as possible like a variable"? They are no variables, why would you want to confuse anybody to think they are? – deetz Jul 20 '17 at 13:12
  • @deetz - Well, @ property seems quite popular in Python. And that's a way to invisibility transform variable access into a function call. – Omnifarious Jul 20 '17 at 14:59
6
  1. That is a good style if we just want to represent pure data.

  2. I don't like it :) because get_/set_ is really unnecessary when we can overload them in C++.

  3. STL uses this style, such as std::streamString::str and std::ios_base::flags, except when it should be avoided! when? When method's name conflicts with other type's name, then get_/set_ style is used, such as std::string::get_allocator because of std::allocator.

Sadeq
  • 7,795
  • 4
  • 34
  • 45
  • 4
    I fear STL might be a bit inconsistent in that respect. Sadly, Stroustrup himself doesn't seem to have voiced an opinion on how to do getters/setters except that he dislikes classes with many of them. – Noarth Sep 06 '10 at 04:35
4

In general, I feel that it is not a good idea to have too many getters and setters being used by too many entities in the system. It is just an indication of a bad design or wrong encapsulation.

Having said that, if such a design needs to be refactored, and the source code is available, I would prefer to use the Visitor Design pattern. The reason is:

a. It gives a class an opportunity to decide whom to allow access to its private state

b. It gives a class an opportunity to decide what access to allow to each of the entities who are interested in its private state

c. It clearly documents such exteral access via a clear class interface

Basic idea is:

a) Redesign if possible else,

b) Refactor such that

  1. All access to class state is via a well known individualistic interface

  2. It should be possible to configure some kind of do's and don'ts to each such interface, e.g. all access from external entity GOOD should be allowed, all access from external entity BAD should be disallowed, and external entity OK should be allowed to get but not set (for example)

Chubsdad
  • 24,777
  • 4
  • 73
  • 129
  • Interesting point indeed, no idea why this was voted down. But I think I'd prefer the friend keyword if a specific class would need access to specific members. – fhd Sep 23 '10 at 18:34
2
  1. I would not exclude accessors from use. May for some POD structures, but I consider them a good thing (some accessors might have additional logic, too).

  2. It doesn't realy matters the naming convention, if you are consistent in your code. If you are using several third party libraries, they might use different naming conventions anyway. So it is a matter of taste.

Cătălin Pitiș
  • 14,123
  • 2
  • 39
  • 62
2

I've seen the idealization of classes instead of integral types to refer to meaningful data.

Something like this below is generally not making good use of C++ properties:

struct particle {
    float mass;
    float acceleration;
    float velocity;
} p;

Why? Because the result of p.mass*p.acceleration is a float and not force as expected.

The definition of classes to designate a purpose (even if it's a value, like amount mentioned earlier) makes more sense, and allow us to do something like:

struct amount
{
    int value;

    amount() : value( 0 ) {}
    amount( int value0 ) : value( value0 ) {}
    operator int()& { return value; }
    operator int()const& { return value; }
    amount& operator = ( int const newvalue )
    {
        value = newvalue;
        return *this;
    }
};

You can access the value in amount implicitly by the operator int. Furthermore:

struct wage
{
    amount balance;

    operator amount()& { return balance; }
    operator amount()const& { return balance; }
    wage& operator = ( amount const&  newbalance )
    {
        balance = newbalance;
        return *this;
    }
};

Getter/Setter usage:

void wage_test()
{
    wage worker;
    (amount&)worker = 100; // if you like this, can remove = operator
    worker = amount(105);  // an alternative if the first one is too weird
    int value = (amount)worker; // getting amount is more clear
}

This is a different approach, doesn't mean it's good or bad, but different.

gus
  • 1
  • 2
1

An additional possibility could be :

int& amount();

I'm not sure I would recommend it, but it has the advantage that the unusual notation can refrain users to modify data.

str.length() = 5; // Ok string is a very bad example :)

Sometimes it is maybe just the good choice to make:

image(point) = 255;  

Another possibility again, use functional notation to modify the object.

edit::change_amount(obj, val)

This way dangerous/editing function can be pulled away in a separate namespace with it's own documentation. This one seems to come naturally with generic programming.

ozcanovunc
  • 703
  • 1
  • 8
  • 29
log0
  • 10,489
  • 4
  • 28
  • 62
  • I didn't vote this down, but the very first is a very poor answer because it exposes design details that a regular accessor keeps hidden. And the other suggestion isn't worthwhile enough to redeem the answer. If you're going to attempt something like the first style, at least return some kind of object that has an `operator =` instead of a reference to an internal data member. – Omnifarious Sep 06 '10 at 06:02
  • To make things clear, my answer only completes the list of possibilities the OP started (I should have comment maybe). I do not especially recommend those possibilities. Like Omnifarious said using a proxy class overloading `operator=` in the first case is obviously a good thing to do at minimum. I would use the second one in some case if I want to provide editing functionality transverse to the class hierarchy, and when it makes sens to clearly separate this functionality from the class. It is probably a bad idea for a simple setter though. – log0 Sep 06 '10 at 11:55
0

Let me tell you about one additional possiblity, which seems the most conscise.

Need to read & modify

Simply declare that variable public:

class Worker {
public:
    int wage = 5000;
}

worker.wage = 8000;
cout << worker.wage << endl;

Need just to read

class Worker {
    int _wage = 5000;
public:
    inline int wage() {
        return _wage;
    }
}

worker.wage = 8000; // error !!
cout << worker.wage() << endl;

The downside of this approach is that you need to change all the calling code (add parentheses, that is) when you want to change the access pattern.

Rok Kralj
  • 46,826
  • 10
  • 71
  • 80
0

variation on #3, i'm told this could be 'fluent' style

class foo {
  private: int bar;
  private: int narf;
  public: foo & bar(int);
  public: int bar();
  public: foo & narf(int);
  public: int narf();
};

//multi set (get is as expected)
foo f; f.bar(2).narf(3);
  • This is ok in terms of functionality but somewhat beyond the scope of the question. This is basically the c# style getter/setter with the ability to chain sets. I`ve heard this pattern is not often used and I know it is certainly easily replaceable. The question also asks for an explanation for *why* you use this pattern over others, if you would care to give one. – Matias Chara Jul 24 '20 at 00:23
  • Sidenote: you don't need to repeat private and public before every single field/method. Put all the private members after a private: and all public members after a public: – Barnack Mar 09 '21 at 17:44