How to correctly move ownership from raw pointer to std::unique_ptr?

Question

My approach is:

class SomeClass
{
    std::vector<std::unique_ptr<MyObject>> myObjects;
public:
    void takeOwnership(MyObject *nowItsReallyMyObject)
    {
        myObjects.emplace_back(std::move(nowItsReallyMyObject));
    }
};

Am I doing everything correctly or are there any better solutions?

Answer 1

The move is redundant.

Myself, I'd do this:

void takeOwnership(std::unique_ptr<MyObject> nowItsReallyMyObject)
{
    myObjects.emplace_back(std::move(nowItsReallyMyObject));
}

because I would want to move the unique_ptr ownership semantics as far "out" as possible.

I might write this utility function:

template<class T>
std::unique_ptr<T> wrap_in_unique( T* t ) {
  return std::unique_ptr<T>(t);
}

so callers can:

foo.takeOwnership(wrap_in_unique(some_ptr));

but even better, then can push the borders of unique_ptr semantics out as far as they reasonably can.

I might even do:

template<class T>
std::unique_ptr<T> wrap_in_unique( T*&& t ) {
  auto* tmp = t;
  t = 0;
  return std::unique_ptr<T>(tmp);
}
template<class T>
std::unique_ptr<T> wrap_in_unique( std::unique_ptr<T> t ) {
  return std::move(t);
}

which lets callers transition their T* into unique_ptrs easier. All of their T*->unique_ptr<T> is now wrapped in a std::move, and zeros the source pointer.

So if they had

struct I_am_legacy {
  T* I_own_this = 0;
  void GiveMyStuffTo( SomeClass& sc ) {
    sc.takeOwnership( wrap_in_unique(std::move(I_own_this)) );
  }
};

the code can be transformed into:

struct I_am_legacy {
  std::unique_ptr<T> I_own_this;
  void GiveMyStuffTo( SomeClass& sc ) {
    sc.takeOwnership( wrap_in_unique(std::move(I_own_this)) );
  }
};

and it still compiles and works the same. (Other interaction with I_own_this may have to change, but part of it will already be unique_ptr compatible).

Answer 2

You should accept the unique_ptr from the get-go:

class SomeClass
{
    std::vector<std::unique_ptr<MyObject>> myObjects;
public:
    // tells the world you 0wNz this object
    void takeOwnership(std::unique_ptr<MyObject> myObject)
    {
        myObjects.push_back(std::move(myObject));
    }
};

This way you make it clear you take ownership and you also help other programmers to avoid using raw pointers.

Further reading: CppCoreGuidelines R.32