Reputation: 3344
I have a class that is queried for an internal state object:
class State {...}; //Has a copy and move constructor
class Processor
{
private:
std::unique_ptr<State> state;
public:
void process(...)
{
State newState;
... //create this new state
state.reset(new State(newState));
}
State getState()
{
return std::move(*state.release());
}
};
Is this an appropriate use of std::move
? I can guarantee that getState
will only be called once per call to process
, but because of the design of this particular system I can't just return newState
from process
. Many of the other answers on Stack Overflow and elsewhere said that it's better to just return the object, because the compiler will move it or RVO it if it can anyway, but those were all in the case that the returned object was local to the function.
I don't necessarily need the state object to be behind a unique_ptr, but that seemed like the easiest way to do manage the new state objects. My actual implementation has a pointer being transferred directly to the unique_ptr at the end.
Upvotes: 2
Views: 208
Reputation: 319
You could just return the state
whose type is std::unique_ptr<State>
, it would be perfectly moved with std::move(state)
. In this case you are free from copying the whole state
object when it's returned by value. And the state
object would be automaticaly destroyed if it's not captured by the caller. It's quite efficient approach for complex heap-allocated objects. But be careful, by moving state
you are clearing the state of the Processor
object. So, the getState
isn't a best name in this case, it should be like fetchState
or popState
.
Upvotes: 0
Reputation: 3344
It turns out that the original demo code is buggy- the unique_ptr never frees the pointer. The answer involves moving onto the local function space and then returning normally.
class State {...}; //Has a copy and move constructor
class Processor
{
private:
std::unique_ptr<State> state;
public:
void process(...)
{
State* newState;
... //newState is allocated on the heap somehow
state.reset(newState);
}
State getState()
{
State _state(std::move(*state));
//Optionally: state.reset();
return _state;
}
};
Upvotes: 1
Reputation: 59841
What's wrong with just returning state
struct state {};
class processor {
public:
void process() {
state_ = State();
}
state get_state() {
return std::move(state_);
}
private:
state state_;
};
This will default-construct state_ when a processor is constructed but
you can prevent that with an optional
wrapper. You still have to
guarantee that get_state
is only called after process
, which
outright sucks.
Upvotes: 0