Question

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.

Was it helpful?

Solution

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.

OTHER TIPS

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.

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");

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.

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.)

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.

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

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,

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top