Correct usage of unique_ptr in class member
Solution 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:
- Raw pointers can't be moved.
- The result of the
generateMyType()
expression is already an rvalue anyways.
Solution 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.
EMBEDONIX
Protecting yourself against thrown exceptions, considered as a defensive programming practice.
Updated on October 02, 2020Comments
-
EMBEDONIX over 3 years
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; }
-
emlai about 7 yearsYou don't need the
std::move
there, raw pointers can't be moved. -
EMBEDONIX about 7 years@tuple_cat But this compiles and runs flawlessly
-
πάντα ῥεῖ about 7 years@SaeidYazdani You don't need it though. Rather use
std::make_unique()
. -
EMBEDONIX about 7 years@πάνταῥεῖ Then what would be the return type of the generator function?
-
πάντα ῥεῖ about 7 years@SaeidYazdani
std::unique_ptr<MyType>
.
-
-
eerorika about 7 yearsNote that the question is tagged C++11, so it might be worth mentioning that
std::make_unique
wasn't introduced until C++14.