Correct usage of unique_ptr in class member

Question

I am trying to really move from c++98 to c++11 and newer. I have wrapped my head over most of the new stuff but I am still not sure about the correct usage of unique_ptr.

Consider the example below, where class A has a unique_ptr member (I would have used raw pointer before!). This member variable should be assigned, when user needs, by calling a function somewhere else (not part of the class). Is this the correct usage? If not, what is the best alternative?

class A {
private:
   unique_ptr<MyType> mt;
public:
   void initStuff() {
      mt.reset(std::move(StaticFuncSomewhereElese::generateMyType()));
   } 
};

MyType* StaticFuncSomewhereElese::generateMyType() {
    MyType* temp = new MyType(...);
    //do stuff to temp (read file or something...)
    return temp;
}

Pedicle

You don't need the std::move there, raw pointers can't be moved. – Glynn

@tuple_cat But this compiles and runs flawlessly – Pedicle

@SaeidYazdani You don't need it though. Rather use std::make_unique(). – Motherwell

@πάνταῥεῖ Then what would be the return type of the generator function? – Pedicle

@SaeidYazdani std::unique_ptr<MyType>. – Motherwell

Answer 1

Your code works fine (although the redundant* move can be omitted) but it would be better to construct the unique_ptr as early as possible:

class A {
private:
   std::unique_ptr<MyType> mt;
public:
   void initStuff() {
      mt = StaticFuncSomewhereElese::generateMyType();
   } 
};

std::unique_ptr<MyType> StaticFuncSomewhereElese::generateMyType() {
    auto temp = std::make_unique<MyType>(…);
    // `make_unique` is C++14 (although trivially implementable in C++11).
    // Here's an alternative without `make_unique`:
    // std::unique_ptr<MyType> temp(new MyType(…));

    //do stuff to temp (read file or something...)
    return temp;
}

This way it is clear that the return value of generateMyType must be deleted by the caller, and there's less possibility for memory leaks (e.g. if generateMyType returns early).

* The move is redundant because:

1. Raw pointers can't be moved.
2. The result of the generateMyType() expression is already an rvalue anyways.

Glynn

Note that the question is tagged C++11, so it might be worth mentioning that std::make_unique wasn't introduced until C++14. – Janeth

Answer 2

Is this the correct usage?

Besides std::move being redundant, yes this is correct. It is redundant because a) Bare pointers are copied, whether they are lvalues or rvalues and b) The function doesn't return a reference, so the return value is already an rvalue so there is no need to convert.

But there is room for improvement. In particular, I recommend to return a unique pointer from the factory function:

std::unique_ptr<MyType> StaticFuncSomewhereElese::generateMyType()

This prevents temp from leaking if the initialization throws an exception, and makes it much harder for the user of the factory to accidentally leak the returned pointer.

Janeth

Answer 3

Why not make it a generic template factory?

In header.

template <typename T>
std::unique_ptr<T> generateMyType();

classss A {
private:
   std::unique_ptr<MyType> mt;
   std::unique_ptr<MyOtherType> mot;
public:
   void initStuff() {
      mt = generateMyType<MyType>();
      mot = generateMyType<MyOtherType>();
   } 
};

And in the source file

template <typename T>
std::unique_ptr<T> generateMyType()
{
  auto temp = std::make_unique<T>();
  return temp;
}

Communism

Answer 4

Also, I would like to add that you may need to define copy constructor and assignment operator for your class.

I used a vector of such class, std::vector<MyClass>, and run into a problem. It feels obvious when you think about it as vector needs a way to copy the data over if it needs to relocate. You can not trivially copy the member that is a unique_ptr, thus the need to write the copy constructor for the class containing smart pointer.

I wrote a non-const copy constructor to move the pointer to resolve my issue.

Douceur

Actually, all you need is a move constructor for MyClass. Writing a copy constructor for a class containing a unique_ptr is somewhat complicated -- for starters, you have to define what it actually should do. Writing a move constructor, on the other hand, is trivial. – Ment

I will look at writing the move contstructor instead. Thanks for the information. I felt the discussion for the class containing unique_ptr was needed as I did not see this caveat being pointed out. – Douceur