Consider the following scenario.
We have a class bar
that owns a foo
data object via a unique pointer. A bar
must always be constructible into a valid state; that is, bar
must always own some data, so data
should never be null. We can call get_data()
to peer into the data of bar
, but the ownership of this data should not change for as long as bar
is alive. So, data is added to bar
when we construct it: the constructor takes in the unique pointer via move semantics.
Then we have baz
, which can own multiple foo
data. We can add each data one at a time by using add_data()
, using std::move()
to move the data into baz
. So far, so good.
struct foo {};
class bar
{
public:
bar(std::unique_ptr<foo> data) : data{ std::move(data) }
{
if (!this->data)
{
throw std::invalid_argument{ "Unexpected null pointer" };
}
}
foo* get_data() { return (this->data).get(); }
private:
std::unique_ptr<foo> data;
};
class baz
{
public:
baz() = default;
void add_data(std::unique_ptr<foo> data) { vec.push_back(std::move(data)); }
private:
std::vector<std::unique_ptr<foo>> vec;
};
int main()
{
// bar invalid_bar{ std::unique_ptr<foo>() }; // throws an exception
std::unique_ptr<foo> data_0{ std::make_unique<foo>() };
bar bar_0{ std::move(data_0) };
std::unique_ptr<foo> data_1{ std::make_unique<foo>() };
std::unique_ptr<foo> data_2{ std::make_unique<foo>() };
std::unique_ptr<foo> data_3{ std::make_unique<foo>() };
baz baz_0;
baz_0.add_data(std::move(data_1));
baz_0.add_data(std::move(data_2));
baz_0.add_data(std::move(data_3));
}
Now I want to be able to merge the data in bar
into that of baz
:
void merge_data(baz& target, bar&& source)
{
// ...
}
Contrary to the provided example code, these classes may instead represent large data structures, in which case unnecessary copying would best be avoid. The intention for this function is to take in a bar
by move semantics, and whatever data was stored on it is now transferred to baz
.
The problem is, this requires extracting the data from bar
, along with its ownership. This means leaving a bar
in an invalid null state. This extraction is fine if it happens inside merge_data()
, since the bar
is moved in, so the resulting invalid bar
shouldn't see the light of day. However, extracting this data will involve adding a member function which is accesible to merge_data()
. So, I consider adding the following public member to bar
:
std::unique_ptr<foo> extract_data() { return std::move(this->data); }
This is ok, but there is one problem with this: since this is public, this means that there is a risk it could be called in the middle of the lifetime of a foo
, risking the possibility of having foo
in an invalid null state in the middle of use. I might be aware that calling this function will leave an invalid bar
, and keep in mind to simply not use the bar
after doing so. But I suspect it will be an unfortunate surprise to other uses who see the function and think that the extracted data is only being copied from bar
!
How can I allow this extract_data()
operation while avoiding leaving unsuspecting invalid bar
objects?
According to your answer in the comments, whether moving bar
leaves it in a valid state:
I consider it here to be invalid; it doesn't uphold the invariant of the class, where it holds a non-null unique pointer. [...] Still, the fact it has been moved from should suggest to the user "this is in an unspecified state"
You can simply add the ref-value qualifier &&
to extract_data
:
std::unique_ptr<foo> extract_data() && { return std::move(this->data); }
This way, extract_data
can only be called on rvalues of bar
.
bar bar_0{std::make_unique<foo>()};
// does not compile, user may be tempted to use bar afterwards
bar_0.extract_data();
// compiles, user has to move bar first, unspecified state afterwards
std::move(bar_0).extract_data();