This is my simple unique_ptr implementation. Anything that could be improved upon or should be added?
#include <algorithm>
template<typename T>
class unique_ptr
{
private:
T * ptr_resource = nullptr;
public:
// Safely constructs resource. Operator new is called by the user. Once constructed the unique_ptr will own the resource.
// std::move is used because it is used to indicate that an object may be moved from other resource.
explicit unique_ptr(T* raw_resource) noexcept : ptr_resource(std::move(raw_resource)) {}
unique_ptr(std::nullptr_t) : ptr_resource(nullptr) {}
// destroys the resource when object goes out of scope
~unique_ptr() noexcept
{
delete ptr_resource;
}
// Disables the copy/ctor and copy assignment operator. We cannot have two copies exist or it'll bypass the RAII concept.
unique_ptr(const unique_ptr<T>&) noexcept = delete;
unique_ptr& operator = (const unique_ptr&) noexcept = delete;
public:
// releases the ownership of the resource. The user is now responsible for memory clean-up.
T* release() noexcept
{
T* resource_ptr = this->ptr_resource;
this->ptr_resource = nullptr;
return resource_ptr;
}
// returns a pointer to the resource
T* get() const noexcept
{
return ptr_resource;
}
// swaps the resources
void swap(unique_ptr<T>& resource_ptr) noexcept
{
std::swap(ptr_resource, resource_ptr.ptr_resource);
}
// replaces the resource. the old one is destroyed and a new one will take it's place.
void reset(T* resource_ptr) noexcept(false)
{
// ensure a invalid resource is not passed or program will be terminated
if (resource_ptr == nullptr)
throw std::invalid_argument("An invalid pointer was passed, resources will not be swapped");
delete ptr_resource;
ptr_resource = nullptr;
std::swap(ptr_resource, resource_ptr);
}
public:
// operators
T* operator->() const noexcept
{
return this->ptr_resource;
}
T& operator*() const noexcept
{
return *this->ptr_resource;
}
// May be used to check for nullptr
};
-
\$\begingroup\$ Please read my series on smart pointers for some help: lokiastari.com/series \$\endgroup\$Loki Astari– Loki Astari2019年05月06日 19:18:49 +00:00Commented May 6, 2019 at 19:18
1 Answer 1
Let me first assume that your
unique_ptr
is supposed to be movable. Then, any basic test case whould have unrevealed this:unique_ptr<int> ptr1(new int()); unique_ptr<int> ptr2 = std::move(ptr1); // Fails to compile
Recall that
= delete
-ing special member functions means user-declaring them. And user-declared copy and copy assignment constructors prevent compiler-generated move (assignment) constructors! You have to manually define them. This case is by the way covered by the rule of five/C.21 Core Guidelines, and also have a look at the table posted in this SO answer for an overview of compiler-generated/-deleted/not-declared special member functions.Is the non-availability of an implicit conversion to
bool
intended? Checking if a (smart) pointer is in an empty/null state is so common in ordinary control flow statements that clients will expect this to compile:unique_ptr<SomeType> ptr = ...; if (ptr) ... // currently fails to compile
But not that this might be debatable. Implicit conversions can cause a lot of pain, so if you intend to not allow them for the sake of a more explicit
if (ptr == nullptr) ...
that's a design decision. But one that should be documented in a comment at the top of the class.
Except the non-
explicit
-ness of the second constructor (thanks to @Deduplicator for pointing that out) taking astd::nullptr_t
, it is superfluous. You can construct an emptyunique_ptr
byunique_ptr<SomeType> empty{nullptr};
which simply invokes the first constructor taking a
T*
argument. I would remove the second constructor.... and add a default constructor that initializes
ptr_resource
tonullptr
, asunique_tr<SomeType> empty;
might be a way of constructing an empty smart pointer that users would expect to compile.
Move-constructing the
ptr_resource
in the constructor initializer byptr_resource(std::move(raw_resource))
doesn't make much sense. Just copy the pointer instead. The comment// std::move is used because it is used to indicate that an object may be moved from other resource.
is rather confusing, becauseT* raw_resource
is already a pointer, and hence a handle to a resource, not the resource itself.The
release
member function can be implemented more conveniently asT* release() noexcept { return std::exchange(ptr_resource, nullptr); }
I wouln't let the
reset
member function throw when the input is anullptr
. Why shouldn't it be allowed toreset
aunique_ptr
with anullptr
, turning it back into an empty state?The only facilities you use from the standard library are
std::move
andstd::swap
. Those are in<utility>
, so you don't need to include<algorithm>
, which is probably much heavier in terms of compile times.I would omit the
this->
prefix, it's unnecessarily verbose. But that might be a matter of taste.Have you considered custom deleters? This makes the class template more reusable in scenarios other than pointers to heap resources, e.g. closing a file upon destruction etc.
Explore related questions
See similar questions with these tags.