Otdfbt

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
;

• 
  

  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 a std::nullptr_t, it is superfluous. You can construct an empty unique_ptr by

  
  
  

  unique_ptr<SomeType> emptynullptr;
  

  
  
  

  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 to nullptr, as

  
  
  

  unique_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 by ptr_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, because T* 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 as

  
  
  

  T* release() noexcept
  
   return std::exchange(ptr_resource, nullptr);
  
  

  
  



• I wouln't let the reset member function throw when the input is a nullptr. Why shouldn't it be allowed to reset a unique_ptr with a nullptr, turning it back into an empty state?




• The only facilities you use from the standard library are std::move and std::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.