CODEWITHSUNDEEP
c++
Dimitrios Menounos
Dimitrios Menounos

Reputation: 555

How to properly return reference to class member?

class Foo {
    protected:
    QPoint& bar() const;

    private:
    QPoint m_bar;
};

QPoint& Foo::bar() const {
    return m_bar;
}

I got this error:

error: invalid initialization of reference of type ‘QPoint&’ from expression of type ‘const QPoint’

However it works if I change it to this:

QPoint& Foo::bar() const {
    return (QPoint&) m_bar;
}

1) I don't understand why the compiler says that my QPoint is const.

2) Is it ok to leave the cast there?

Upvotes: 17

Views: 27346

Answers (5)

sbi
sbi

Reputation: 224039

In a non-const member function of class Foo the this pointer is of the type Foo* const - that is, the pointer is const, but not the instance it points to. In a const member function, however, the this pointer is of the type const Foo* const. This means that the object it points to is constant, too.

Therefore, in your example, when you use this->m_bar (of which m_bar is just the short form), then m_bar is a member of a constant object - which is why you cannot return it as a non-const reference.

This actually makes sense from a design POV: If that Foo object is a constant object, and you are allowed to invoke Foo::bar for constant objects, then, if this would return a non-const reference to some internals which you can fiddle with, you would be able to change the state of a constant object.

Now you have to look at your design and ask yourself how you arrived at this point and what your actual intention is. If that m_bar member isn't really part of the state of the object (for example, it's only there for debugging purposes), then you could consider making it mutable. If it is part of the state of the object, then you have to ask yourself why you want to return a non-const reference to some internal data of a constant object. Either make the member function non-const, or return a const reference, or overload the member function: 

class Foo {
public:
  const QPoint& bar() const {return m_bar;}
        QPoint& bar()       {return m_bar;}
  // ...
};

Upvotes: 26

KitsuneYMG
KitsuneYMG

Reputation: 12901

either use

const QPoint& Foo::bar() const {
    return m_bar;
}

or

QPoint& Foo::bar() {
    return m_bar;
}

I guess you could also declare m_bar as:

mutable QPoint m_bar;

EDIT: mutable defense

@tstenner:
mutable has its place. It is not 'evil' definitely no more so than void* or casting. Assume something like the following:

class foo {
    SomeType bar;

public:
    foo() : bar() { }

    const SomeType& bar() const { return a; }
};

In this case, bar is ALWAYS constructed, even if bar() is never called. This may be fine, but if SomeType has a costly constructor, it can be preferable to allow lazy instantiation of bar.

Consider:

class foo2 {
    mutable SomeType* bar;
    mutable bool cached;

public:
    foo2() : bar(0), cached(false) { }

    const SomeType& bar() const {
        if( ! cached ) {
            cached = true;
            bar = new SomeType();
        }
        return *bar;
    }
};

This allows foo2 to mimic a non-lazy instantiated class, while being lazily instantiated. Granted, the mutability of foo2 has implications for thread safety, but these can be overcome with locking if needed.

Upvotes: 4

RC.
RC.

Reputation: 28197

What you're trying to do is a no-no. You do not want to return a QPoint & from your bar function as this breaks encapsulation because a caller can now modify m_bar out from underneath QPoint. (luckily you have the function declared as const or you wouldn't get an error here and you would still be breaking encapsulation).

What you want in this case is

const QPoint &Foo::bar() const
{
    return m_bar;
}

Edit based on user comment:

IMHO, this would be better solved by adding a setX to Foo instead of calling a non-const function from a non-const reference returned by an accessor function that should really be const. Overloading the accessor function to remove the const just puts a band-aid over the real problem and just hides the fact that encapsulation is compromised. Adding setX to Foo fixes the encapsulation problem and also plugs the leaking interface you are creating by returning a non-const reference to private data.

For example, if you put foo.bar().setX(100); in many parts of your application and later change the type of QPoint to one that doesn't implement setX or simply rename the function setX to say setPointX you have problems b/c you now have to go around a rename/refactor all of these calls. Creating a setX on Foo makes the code easier to call foo.setX(100) vs foo.bar().setX(100), is const correct, and encapsulates your data. If you changed QPoint to be just 2 x and y coordinates in Foo, nothing outside of your class would have to change b/c you have good encapsulation.

Upvotes: 5

P Shved
P Shved

Reputation: 99254

If it allowed you to return a reference to the variable, then you could modify a const instance of Foo class. Consider such code:

void baz(const Foo &foo)
{
  QPoint &ref = foo.bar();  //if it was allowed...
  ref = 5; // ...you could have modified a const object foo!
}

Therefore compiler prevents you from doing that.

You should either declare your method returning const QPoint& or to revise your understanding of what really const is for.

Someone may advise you to use mutable. Don't. mutable keyword is to allow implementation of the class to modify internal member variables of const objects (for example, for memoization purposes), rather than to expose non-const variable (!) to external code.

Upvotes: 2

Lodle
Lodle

Reputation: 32187

The const on the function tells the compiler that the function doesnt modify any member variables. However, since your returning a reference to the member variable it cant guarantee that any more and thus has the compile error.

The compiler is not expecting your member variable to be const but the return from the function to be const. Remove the const from the function def.

Upvotes: 3

Related Questions