We can use nodiscard
attribute to imply that the return value of a function should not be discarded. Are there any attribute (or other ways) to imply some opposite semantics: the return value of the function should only be used temporarily (by "temporary" I mean, not to assign to any variable except local ones)?
As the purpose may not be immediately clear, consider I have a class FooHolder
that holds resources Foo
; calling FooHolder::getFoo()
returns the Foo
it is currently holding:
#include <memory>
class Foo {
public:
Foo& bar() { /* do something */ return *this; }
const Foo& far() const { /* do something else */ return *this; }
};
class FooHolder {
private:
std::shared_ptr<Foo> _foo { nullptr };
public:
FooHolder(): _foo(std::make_shared<Foo>()) {}
Foo& getFoo() { return *_foo; }
const Foo& getFoo() const { return *_foo; }
};
And we may use it in many ways:
// Others may try storing some status:
Foo* g_foo = nullptr;
int main() {
FooHolder foo_holder {};
// I want to support this:
foo_holder.getFoo().bar().far() /* chained calls... */ ;
// Also, maybe this:
auto& local_foo = foo_holder.getFoo();
local_foo.bar();
local_foo.far();
// But not this, because the Foo instance that FooHolder holds may perish:
static Foo& static_foo = foo_holder.getFoo();
// Nor this:
g_foo = &local_foo;
return 0;
}
So are there ways to prevent (or at least warn about) storing the return value of FooHolder::getFoo()
? Or, is it bad practice to return resources by reference?
CodePudding user response:
[...] is it bad practice to return resources by reference?
It depends. There are many examples of methods that return non-const references and they are all fine. For example consider standard container element accessors. However, they are not meant for encapsulation. std::vector::operator[]
is not meant to hide the elements from the caller, it is meant to provide direct access to it. Returning a non-const reference is not encapsulation! It is the opposite. Note that std::vector
even grants you access to its data()
. Thats not encapsulation either, it is relying on the user to not delete[] some_vect.data()
or do other wrong stuff that would break the vector.
You want FooHolder
to encapsulate the contained Foo
.
This is opposing requirements.
You have basically two choices: A) The caller knows what they are doing. They read documentation. They know that they are not supposed to use FooHolder::getFoo
in the wrong way. B) use proper encapsulation: Never give the caller direct access to a non-const Foo
:
class FooHolder {
private:
std::shared_ptr<Foo> _foo { nullptr };
public:
FooHolder(): _foo(std::make_shared<Foo>()) {}
// nope // Foo& getFoo() { return *_foo; }
// maybe // const Foo& getFoo() const { return *_foo; }
FooHolder& bar() {
_foo->bar();
return *this;
}
// ..same for far() ...
};
Note that A) is a viable solution. Consider that also things like std::shared_ptr
can be used terribly wrong. Users are expected to know how to use it right. The difference is that std::shared_ptr
is a standard type with plenty of documentation. Hence, you should think twice if this is the way to go.
So are there ways to prevent (or at least warn about) storing the return value of FooHolder::getFoo()?
No. Once you returned a non-const reference all bets are off. FooHolder
is no longer in control of what the caller can do with that reference. You can prevent copies or moves but you cannot prevent holding on to a reference.