C++ getters/setters coding style
Asked Answered
N

8

71

I have been programming in C# for a while and now I want to brush up on my C++ skills.

Having the class:

class Foo
{
    const std::string& name_;
    ...
};

What would be the best approach (I only want to allow read access to the name_ field):

  • use a getter method: inline const std::string& name() const { return name_; }
  • make the field public since it's a constant

Thanks.

Nunnally answered 17/4, 2009 at 15:4 Comment(9)
Duplicate of #737909 ?Linker
As an aside, it's more common to use a single leading underscore for member variables in C++.Novosibirsk
I also thought that the trailing underscore is kind of awkward, but that's what I saw being used in the C++ Faq Lite.Nunnally
See the following for a pretty complete answer on use of underscores in C++ #229283Hyperesthesia
@MartinBeckett Single underscores may be common, but as a partially sighted person I can tell you they are a pain to read. IDEs just love to underline stuff (errors, spelling, etc.) and the underscore get completely obscured by the "helpfulness". To the coding community at large, hear us blind folks, and put an m_ instead of _.Venavenable
@WesMiller, thank you I never thought of that. Although modern IDEs will allow a local style. We just adopted trailing underscores (following some standard) and they are bad when it comes to pointer_-> or element_[i]Novosibirsk
I hate when people put leading underscores or m_ in Javascript and Java. People seem to forget these are the least inelegant solutions to C++ inconveniences (for example, in Java, fields and methods can have the same name)Hagi
@WesMiller I find all the underscores difficult to read too, despite decent vision. Nowhere have I found more painful-to-read code than some standard C++ libraries.Hagi
@Hagi That's because you're looking in completely the wrong place. Standard libraries are not supposed to be examples of pretty code; they are only supposed to be working code. And there's the point. Stdlib authors have to use ugly variable names, especially in the case of templates, as they have to ensure their identifiers are not interfered with by users, so their code will run as expected (if the user follows the Standard). That's an example of why identifiers containing double-underscores, or particular combinations of leading underscores, are reserved for the implementation to use.Tantalizing
P
49

It tends to be a bad idea to make non-const fields public because it then becomes hard to force error checking constraints and/or add side-effects to value changes in the future.

In your case, you have a const field, so the above issues are not a problem. The main downside of making it a public field is that you're locking down the underlying implementation. For example, if in the future you wanted to change the internal representation to a C-string or a Unicode string, or something else, then you'd break all the client code. With a getter, you could convert to the legacy representation for existing clients while providing the newer functionality to new users via a new getter.

I'd still suggest having a getter method like the one you have placed above. This will maximize your future flexibility.

Pontine answered 17/4, 2009 at 15:8 Comment(5)
If I change from a function that returns a std::string to a function that returns some Unicode string, I'm breaking all client code already.Paternal
..what if your internal representation was Unicode, but you could convert to UTF8 for compatibility with existing clients? A getter method could do the conversion, but a public field would prohibit this pattern.Fossorial
@Fossorial seems like a case where one might want differently-named getters for the Unicode and UTF8 variants to me. If the internal representation remains the same but the getter type changes, you're breaking compatibility; only point to this is if you think the internal representation will change.Hagi
@Hagi another good reason to have gettors for stuff like this is if there's a natural default form, but also optional other forms. To use different 8-bit encodings, one has the choice of either get_my_string() + get_my_utf8_string() + get_my_cp1252_string(), etc. or having get_my_string(const char* encoding = "utf8").Pontine
@MrFooz yeah...but if available I would just return something like a QString that provides all those conversions itself, so that I don't have to write a bunch of getters for each fieldHagi
D
82

Using a getter method is a better design choice for a long-lived class as it allows you to replace the getter method with something more complicated in the future. Although this seems less likely to be needed for a const value, the cost is low and the possible benefits are large.

As an aside, in C++, it's an especially good idea to give both the getter and setter for a member the same name, since in the future you can then actually change the the pair of methods:

class Foo {
public:
    std::string const& name() const;          // Getter
    void name(std::string const& newName);    // Setter
    ...
};

Into a single, public member variable that defines an operator()() for each:

// This class encapsulates a fancier type of name
class fancy_name {
public:
    // Getter
    std::string const& operator()() const {
        return _compute_fancy_name();    // Does some internal work
    }

    // Setter
    void operator()(std::string const& newName) {
        _set_fancy_name(newName);        // Does some internal work
    }
    ...
};

class Foo {
public:
    fancy_name name;
    ...
};

The client code will need to be recompiled of course, but no syntax changes are required! Obviously, this transformation works just as well for const values, in which only a getter is needed.

Dorotheadorothee answered 17/4, 2009 at 15:26 Comment(4)
Hey, I found your answer before posting my question... it seems to be nearly exactly the same and it appears that you're proposing the type of solution I'm looking for but being a noob I can't be sure. Would you mind having a look at my question here: #8455387 and letting me know if this answer is on the right track for what I'd like to accomplish?Purplish
If you were confused like me: there is no operator()(). There is an operator() with a parameter list.Leigh
Wouldn't the replacement with a public member break the encapsulation allowing bypassing the getter/setter?Bulkhead
@a1an: The idea is that the public member is not some simple type like std::string (which could be changed by the caller in invariant-breaking ways), but rather an object of some class type (like my example fancy_name class) that protects all necessary invariants itself by exposing a more limited interface.Dorotheadorothee
P
49

It tends to be a bad idea to make non-const fields public because it then becomes hard to force error checking constraints and/or add side-effects to value changes in the future.

In your case, you have a const field, so the above issues are not a problem. The main downside of making it a public field is that you're locking down the underlying implementation. For example, if in the future you wanted to change the internal representation to a C-string or a Unicode string, or something else, then you'd break all the client code. With a getter, you could convert to the legacy representation for existing clients while providing the newer functionality to new users via a new getter.

I'd still suggest having a getter method like the one you have placed above. This will maximize your future flexibility.

Pontine answered 17/4, 2009 at 15:8 Comment(5)
If I change from a function that returns a std::string to a function that returns some Unicode string, I'm breaking all client code already.Paternal
..what if your internal representation was Unicode, but you could convert to UTF8 for compatibility with existing clients? A getter method could do the conversion, but a public field would prohibit this pattern.Fossorial
@Fossorial seems like a case where one might want differently-named getters for the Unicode and UTF8 variants to me. If the internal representation remains the same but the getter type changes, you're breaking compatibility; only point to this is if you think the internal representation will change.Hagi
@Hagi another good reason to have gettors for stuff like this is if there's a natural default form, but also optional other forms. To use different 8-bit encodings, one has the choice of either get_my_string() + get_my_utf8_string() + get_my_cp1252_string(), etc. or having get_my_string(const char* encoding = "utf8").Pontine
@MrFooz yeah...but if available I would just return something like a QString that provides all those conversions itself, so that I don't have to write a bunch of getters for each fieldHagi
U
24

As an aside, in C++, it is somewhat odd to have a const reference member. You have to assign it in the constructor list. Who owns the actually memory of that object and what is it's lifetime?

As for style, I agree with the others that you don't want to expose your privates. :-) I like this pattern for setters/getters

class Foo
{
public:
  const string& FirstName() const;
  Foo& FirstName(const string& newFirstName);

  const string& LastName() const;
  Foo& LastName(const string& newLastName);

  const string& Title() const;
  Foo& Title(const string& newTitle);
};

This way you can do something like:

Foo f;
f.FirstName("Jim").LastName("Bob").Title("Programmer");
Unreconstructed answered 18/4, 2009 at 3:7 Comment(3)
Although it reads and writes well, method chaining ('fluent') API style carries some overhead: https://mcmap.net/q/276360/-why-should-an-api-return-39-void-39/102345Fossorial
@Unreconstructed I have been working with C++ many years, but have never thought of doing it that way. What is the name of this getter/setter style?Gambia
@Gambia it's called the fluent api style. It's related to the Java builder pattern.Fermat
P
8

I think the C++11 approach would be more like this now.

#include <string>
#include <iostream>
#include <functional>

template<typename T>
class LambdaSetter {
public:
    LambdaSetter() :
        getter([&]() -> T { return m_value; }),
        setter([&](T value) { m_value = value; }),
        m_value()
    {}

    T operator()() { return getter(); }
    void operator()(T value) { setter(value); }

    LambdaSetter operator=(T rhs)
    {
        setter(rhs);
        return *this;
    }

    T operator=(LambdaSetter rhs)
    {
        return rhs.getter();
    }

    operator T()
    { 
        return getter();
    }


    void SetGetter(std::function<T()> func) { getter = func; }
    void SetSetter(std::function<void(T)> func) { setter = func; }

    T& GetRawData() { return m_value; }

private:
    T m_value;
    std::function<const T()> getter;
    std::function<void(T)> setter;

    template <typename TT>
    friend std::ostream & operator<<(std::ostream &os, const LambdaSetter<TT>& p);

    template <typename TT>
    friend std::istream & operator>>(std::istream &is, const LambdaSetter<TT>& p);
};

template <typename T>
std::ostream & operator<<(std::ostream &os, const LambdaSetter<T>& p)
{
    os << p.getter();
    return os;
}

template <typename TT>
std::istream & operator>>(std::istream &is, const LambdaSetter<TT>& p)
{
    TT value;
    is >> value;
    p.setter(value);
    return is;
}


class foo {
public:
    foo()
    {
        myString.SetGetter([&]() -> std::string { 
            myString.GetRawData() = "Hello";
            return myString.GetRawData();
        });
        myString2.SetSetter([&](std::string value) -> void { 
            myString2.GetRawData() = (value + "!"); 
        });
    }


    LambdaSetter<std::string> myString;
    LambdaSetter<std::string> myString2;
};

int _tmain(int argc, _TCHAR* argv[])
{
    foo f;
    std::string hi = f.myString;

    f.myString2 = "world";

    std::cout << hi << " " << f.myString2 << std::endl;

    std::cin >> f.myString2;

    std::cout << hi << " " << f.myString2 << std::endl;

    return 0;
}

I tested this in Visual Studio 2013. Unfortunately in order to use the underlying storage inside the LambdaSetter I needed to provide a "GetRawData" public accessor which can lead to broken encapsulation, but you can either leave it out and provide your own storage container for T or just ensure that the only time you use "GetRawData" is when you are writing a custom getter/setter method.

Perplexity answered 9/12, 2013 at 16:24 Comment(4)
isnt this much of an overkill? this approach uses nice encapsulation so you only have to maintain the LambdaSetter class, but from a performance perspective , I think getting rid of the function pointers would be a good idea? And also myString.SetGetter( looks to me a bit awkward, by giving the object a function that it is able to perform the given action namely setting the myString. The approach is definitely going in the right direction but I would cleary simplify this, without having the std::functionsGryphon
It's overkill for sure, but what it gains you is the ability to change at runtime how a variable is set/get. Getting rid of the std::functions would turn it back into the set/get being determined at compile time (probably good for most applications) A templated class might serve better for that, so the class would optionally take a set/get method and it would be determined at compile time.Perplexity
I would call this a premature optimization.Individualism
As an aside I would never bother with this, trying to achieve what C# does natively in c++ is a fools errand, but it does answer the OP's question.Perplexity
L
5

Even though the name is immutable, you may still want to have the option of computing it rather than storing it in a field. (I realize this is unlikely for "name", but let's aim for the general case.) For that reason, even constant fields are best wrapped inside of getters:

class Foo {
    public:
        const std::string& getName() const {return name_;}
    private:
        const std::string& name_;
};

Note that if you were to change getName() to return a computed value, it couldn't return const ref. That's ok, because it won't require any changes to the callers (modulo recompilation.)

Lita answered 17/4, 2009 at 15:10 Comment(5)
So let me resummarize this answer: 1) it is highly unlikely that you'll ever change the implementation of getName(), because... hell, the data is just stored, how in the world you will be recomputing it every time it's accessed? You are using C++ for performance, aren't you? and 2) in the end, even if you would really come up with such a crazy usecase, you won't actually be able to do that, because it's impossible to return a const ref without triggering undefined behavior. Are you convinced yet?Adduct
ulidtko, based on your "tone of voice" here, I doubt that any answer I give you could mollify you. However, let me point out one thing: I wrote "Note that if you were to change getName() to return a computed value, it couldn't return const ref. That's ok, because it won't require any changes to the callers (modulo recompilation.)" In other words, you'd have to change the method from const std::string& getName() const to std::string getName() const. Of course, changing an inline function, or a field, always requires recompilation, so this doesn't increase the compile-time burden.Lita
your note here is correct. You could profit from some smart pointers too, probably. However it is true that I have an opinion on getters and setters in C++, and I'm collecting arguments for, and against it.Adduct
Regarding this argument for getters (i.e. an option to switch to recomputing the value instead of returning it stored) I can make the following objection: almost certainly by recomputing the value in getter you'll be breaking a contract. Most of the clients will rely on certain "intuitive" properties of getters: that they're fast, they don't throw, they don't allocate, should be threadsafe, etc. You may get lucky and everything will still work with your untrivial getter, but programming isn't about luck.Adduct
I agree with you that sometimes a caller may be relying on a getter running essentially as quickly as accessing a field. In some circles, this is used as an argument against language support for transparent getters. I'm sympathetic to the idea of an implicit performance contract, but I think a bit of caveat emptor applies as well: If the developer uses accessors, they're reserving the right to change the implementation. And if they didn't explicitly promise constant-time access, then you shouldn't write code that assumes constant-time access.Lita
A
3

Avoid public variables, except for classes that are essentially C-style structs. It's just not a good practice to get into.

Once you've defined the class interface, you might never be able to change it (other than adding to it), because people will build on it and rely on it. Making a variable public means that you need to have that variable, and you need to make sure it has what the user needs.

Now, if you use a getter, you're promising to supply some information, which is currently kept in that variable. If the situation changes, and you'd rather not maintain that variable all the time, you can change the access. If the requirements change (and I've seen some pretty odd requirements changes), and you mostly need the name that's in this variable but sometimes the one in that variable, you can just change the getter. If you made the variable public, you'd be stuck with it.

This won't always happen, but I find it a lot easier just to write a quick getter than to analyze the situation to see if I'd regret making the variable public (and risk being wrong later).

Making member variables private is a good habit to get into. Any shop that has code standards is probably going to forbid making the occasional member variable public, and any shop with code reviews is likely to criticize you for it.

Whenever it really doesn't matter for ease of writing, get into the safer habit.

Aguayo answered 17/4, 2009 at 15:50 Comment(1)
I tend to disagree, because with a public field the "changed requirements" situation is not that fatal: you could still use some templates, inheritance and operator= to simulate a property in C++. And actually most of the time you won't need this (seriously, how many times did you do nontrivial getters or setters?), while the client syntax of get()/set() is just ugly.Adduct
C
1

Collected ideas from multiple C++ sources and put it into a nice, still quite simple example for getters/setters in C++:

class Canvas { public:
    void resize() {
        cout << "resize to " << width << " " << height << endl;
    }

    Canvas(int w, int h) : width(*this), height(*this) {
        cout << "new canvas " << w << " " << h << endl;
        width.value = w;
        height.value = h;
    }

    class Width { public:
        Canvas& canvas;
        int value;
        Width(Canvas& canvas): canvas(canvas) {}
        int & operator = (const int &i) {
            value = i;
            canvas.resize();
            return value;
        }
        operator int () const {
            return value;
        }
    } width;

    class Height { public:
        Canvas& canvas;
        int value;
        Height(Canvas& canvas): canvas(canvas) {}
        int & operator = (const int &i) {
            value = i;
            canvas.resize();
            return value;
        }
        operator int () const {
            return value;
        }
    } height;
};

int main() {
    Canvas canvas(256, 256);
    canvas.width = 128;
    canvas.height = 64;
}

Output:

new canvas 256 256
resize to 128 256
resize to 128 64

You can test it online here: http://codepad.org/zosxqjTX

PS: FO Yvette <3

Cholecystitis answered 25/10, 2018 at 21:4 Comment(0)
H
0

From the Design Patterns theory; "encapsulate what varies". By defining a 'getter' there is good adherence to the above principle. So, if the implementation-representation of the member changes in future, the member can be 'massaged' before returning from the 'getter'; implying no code refactoring at the client side where the 'getter' call is made.

Regards,

Howling answered 17/4, 2009 at 17:4 Comment(1)
encapsulate what varies — translating: "write getters and setters". That kind of argumentation doesn't buy me. While the problem of switching internal representation without breaking client code is solved with properties (which can be emulated in C++), getters and setters syntax IMO is really ugly and forces all the clients to write ugly code. So I'd better make the member variable public, especially when it's const.Adduct

© 2022 - 2024 — McMap. All rights reserved.