CODEWITHSUNDEEP
c++
Daniel Bencik
Daniel Bencik

Reputation: 969

Proper design of member setters/getters in a C++ class

The usual way of designing setters and getters for a class member is the following 

class QNumber 
{
    public:
        void setNumber(unsigned int xNumber){ this->mValue = xNumber; };
        unsigned int getNumber(void) const { return this->mValue; };

    private:
        unsigned int mValue;
}

If the member is another class (e.g. QRational as opposed to unsigned int), then the getter would be better returning a reference, in terms of performance.

So the modified design would be 

class QNumber 
{
    public:
        const QRational & value(void) const  { return  mValue;} // getter
              QRational & value(void)        { return  mValue;} // 'setter'

    private:
        QRational mValue;
}

My question is - isn't there something potentially wrong with the latter approach? I have not seen it much in other people's code and to me, it seems more elegant than set/get method.

Many thanks, Daniel

Upvotes: 0

Views: 128

Answers (3)

Jerry Coffin
Jerry Coffin

Reputation: 490663

At least in my opinion, if that member acts like an unsigned int (e.g., allows assignment to/from an unsigned int), and you're really sure this class should support direct manipulation of that member (i.e., it should have a "getter" and "setter" at all), then you should at least make access to it clean, rather than requiring other code to be written around that implementation detail. To avoid that, you should define how the type of this object differs from a plain unsigned int, then implement that behavior in a class that defines that type properly and directly. 

class QNumber { // bad name--looks like a Qt class name
    unsigned int value;
public:
    QNumber(unsigned int value = 0) : value(value) {}
    QNumber &operator=(unsigned int val) { value = val; return *this; }
    operator unsigned int() { return value; }
};

With this, client code can be readable--instead of ugliness like x.SetNumber(2); or x.SetNumber() = 2; you just use x = 2;. This also avoids all sorts of lifetime issues that arise when you let a client get a pointer or reference to the class' internals (which you should generally avoid).

Upvotes: 2

Petr
Petr

Reputation: 10007

In addition to Potatoswatter's answer, please note one more point.

You second design provokes usage in the following form:

QRational& r = number.value(); 
// or
const QRational& r = number.value();

thus the user retains the reference to your inner object. It will be somewhat more difficult to manage in case your number object can be destroyed or moved while r is still there. This does not even depend on whether the const or non-const method is used.

The first design does not expose such problems.

Upvotes: 2

Potatoswatter
Potatoswatter

Reputation: 137930

The point of getters and setters is to separate the interface from the implementation. If you return a reference, it has to point somewhere in the implementation. Now if you change the implementation, you'll have to start returning by value instead.

Personally I prefer public nonstatic members when the need for anything else is unlikely. Reference-semantic getters and setters provide the worst of both worlds. (I am working on a proposal to improve them, though.)

Upvotes: 3

Related Questions