Is it alright to return a reference to a non-pointer member variable as a pointer?

Question

I recently came across some C++ code that looked like this:

class SomeObject
{
private:
    // NOT a pointer
    BigObject foobar;

public:
    BigObject * getFoobar() const
    {
        return &foobar;
    }
};

I asked the programmer why he didn't just make foobar a pointer, and he said that this way he didn't have to worry about allocating/deallocating memory. I asked if he considered using some smart pointer, he said this worked just as well.

Is this bad practice? It seems very hackish.

Answer 1

That's perfectly reasonable, and not "hackish" in any way; although it might be considered better to return a reference to indicate that the object definitely exists. A pointer might be null, and might lead some to think that they should delete it after use.

The object has to exist somewhere, and existing as a member of an object is usually as good as existing anywhere else. Adding an extra level of indirection by dynamically allocating it separately from the object that owns it makes the code less efficient, and adds the burden of making sure it's correctly deallocated.

Of course, the member function can't be const if it returns a non-const reference or pointer to a member. That's another advantage of making it a member: a const qualifier on SomeObject applies to its members too, but doesn't apply to any objects it merely has a pointer to.

The only danger is that the object might be destroyed while someone still has a pointer or reference to it; but that danger is still present however you manage it. Smart pointers can help here, if the object lifetimes are too complex to manage otherwise.

Answer 2

You are returning a pointer to a member variable not a reference. This is bad design. Your class manages the lifetime of foobar object and by returning a pointer to its members you enable the consumers of your class to keep using the pointer beyond the lifetime of SomeObject object. And also it enables the users to change the state of SomeObject object as they wish.

Instead you should refactor your class to include the operations that would be done on the foobar in SomeObject class as methods.

ps. Consider naming your classes properly. When you define it is a class. When you instantiate, then you have an object of that class.