Question

What is the best practice for a C++ getter method which is supposed to return a non trivial type, but a member which is of type class, or struct.

  1. Return by value, such as: MyType MyClass::getMyType() { return mMyType; }
  2. Return by const reference: const MyType& MyClass::getMyType() { return mMyType; }
  3. Return by address: MyType* MyClass::getMyType() { return &mMyType; }

where

class MyType { /* ... */ };

class MyClass
{
  private:
     MyType mMyType;
}

I specifically worry about the following usages of this method. Can you please elaborate in details how this might affect copying the object, and the danger of dangling references and wild gone pointers if function() wants to save it for further usage.

MyType* savedPointer;

SomeType function(MyType* pointer) { savedPointer = pointer; };

a. valid for 1. and 2.

{
  MyType t = myClass.getMyType();
  function(&t);
}

// is savedPointer still valid here?

b. valid for 1. and 2.

{
  const MyType& t = myClass.getMyType();
  function(&t);
}

// is savedPointer still valid here?

c. valid for 1. and 2.

{
  MyType& t = myClass.getMyType();
  function(&t);
}

// is savedPointer still valid here?

d. valid for 3.

{
  MyType* t = myClass.getMyType();
  function(t);
}

// is savedPointer still valid here?

where myClass is an object of type MyClass.

Was it helpful?

Solution

You can provide both const and non-const versions:

MyType       & MyClass::getMyType()       { return mMyType; }
MyType const & MyClass::getMyType() const { return mMyType; }

I wouldn't provide a pointer version, since that implies that the return value might be the null pointer, which it can never be in this instance.

The real point, however, is that you are basically giving the caller direct access to the internal object. If this is your intent, then you may as well make the data member public. If it isn't, then you will need to work harder to hide the object.

One option is to retain the MyType const & accessor, but provide more indirect means to modify the internal object (setMyType(…) or something more tailored to the semantics that you are trying to express at the level of the containing class).

OTHER TIPS

In general, you should prefer return by value, unless you explicitly want to guarantee that the reference will designate a member (which exposes part of your implementation, but is desirable in cases like std::vector<>::operator[]). Returning a reference prevents later changes in class, since it means that you cannot return a calculated value. (This is especially important if the class is designed to be a base class, since returning a reference creates this restriction for all derived classes.)

The only time you should return by pointer is if a lookup or something is involved, which may return in having to return a null pointer.

Returning a reference to const may be a valid optimization, if the profiler indicates performance problems here, and the call site can also deal with a const reference (no modification of the returned value, no problems with lifetime of object). It must be weighed against the additional constraints on the implementation, of course, but in some cases, it is justified.

I would always return a const reference. If you need to modify the value it is returning just use a setter function.

Return by value, such as: MyType MyClass::getMyType() { return mMyType; } should be avoided as you will copy the content of your object. I do not see the gain you could have but I see the drawbacks on performance.

Return by const reference: const MyType& MyClass::getMyType() { return mMyType; } is more generaly used this way:

const MyType& MyClass::getMyType() const { return mMyType; }
MyType& MyClass::getMyType() { return mMyType; }

Always provide the const version. The non-const version is optional as it means that someone can modify your data. It is what I would encourage you to use.

Return by address: MyType* MyClass::getMyType() { return &mMyType; } is mostly used when the data is optionally there. It often has to be checked before being used.

Now, your use case, I would strongly advice not to keep a pointer save for more than a scope. I can often lead to ownership problems. I you have to do so, take a look to shared_ptr.

For your examples, there is two cases:

a. savedPointer won't be valid any more after the closing brace.

b,c, and d. savedPointer is valid after the closing brace, but beware it should not outlive myClass.

a) MyType t = will create a copy of the object for both 1 and 2. Saved pointer will not be valid once t is out of scope.

b) The saved pointer will be valid for the case returning a reference, but invalid for the case returning an object. For the reference, the lifetime of the pointer would be the same as myClass. Of course &t on a const ref would be a const t* not a t* and so would fail to cast in your call to function(MyType*).

c) Same as b, though the code is invalid for 2 because you can't cast a const MyType& to a MyType&. Generally this would be bad practice and the const form would be more acceptable.

d) savedPointer will have the same lifetime as myClass.

I'd generally lean toward either returning a reference or a const reference depending on what you expect to be able to do with the return value. If you return a reference (non-const), you can do things like: myClass.getMyType() = ..., while if you return a const reference, the object is read-only.

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