6
\$\begingroup\$

This is an implementation to simulate the basic functionality of unique_ptr.
This doesn't provide features like custom deleter and make_unique().

I would really appreciate any feedback to improve the below code, any other api's that I should be providing etc.

my_unique_ptr.h

#ifndef MY_UNIQUE_PTR_H_
#define MY_UNIQUE_PTR_H_

#include <utility>

namespace kapil {
  template <typename T>
  class unique_ptr final {
    private:
      T* ptr_;
      unique_ptr(const unique_ptr&) = delete;               // Make unique_ptr non copy constructible
      unique_ptr& operator = (const unique_ptr&) = delete;  // Make unique_ptr non copy assignable

    public:
      explicit unique_ptr (T* ptr = nullptr) noexcept
        : ptr_{ptr} { }

      unique_ptr(unique_ptr<T>&& rval) noexcept               // Move constructor
        : ptr_{rval.ptr_} {
        rval.ptr_ = nullptr;
      }

      unique_ptr& operator = (unique_ptr&& rhs) noexcept {   // Move assignment
        delete ptr_;
        ptr_ = rhs.ptr_;
        rhs.ptr_ = nullptr;
        return *this;
      }

      ~unique_ptr() noexcept {
        delete ptr_;
      }


      T* release() noexcept {
        T* old_ptr = ptr_;
        ptr_ = nullptr;
        return old_ptr;
      }

      void reset(T* ptr = nullptr) noexcept {
        delete ptr_;
        ptr_ = ptr;
      }

      void swap(unique_ptr& rhs) noexcept {
        std::swap(ptr_, rhs.ptr_);
      }

      T* get() const noexcept {
        return ptr_;
      }

      explicit operator bool() const noexcept {
        return (ptr_ != nullptr);
      }

      T& operator * () const {
        return *ptr_;
      }

      T* operator -> () const noexcept {
        return ptr_;
      }

      friend bool operator == (const unique_ptr& lhs, const unique_ptr& rhs) {
        return lhs.get() == rhs.get();
      }

      friend bool operator != (const unique_ptr& lhs, const unique_ptr& rhs) {
        return !(lhs == rhs);
      }
  };

  template <typename T>
  void swap(unique_ptr<T>& lhs, unique_ptr<T>& rhs) {
    lhs.swap(rhs);
  }
} //kapil

#endif
\$\endgroup\$
5
\$\begingroup\$
  1. Don't mark your class final without a good reason. It inhibits user-freedom.

  2. The default-access for members of a class is already private.

  3. Explicitly deleting copy-constructor and copy-assignment-operator is superfluous, as you define move-constructor and move-assignment, which already suppresses them. Still, some assert being explicit adds clarity.

  4. I wonder why you didn't declare construction from T* to be constexpr...

  5. Try to consistently use the injected class-name (unique_ptr), instead of sporadically naming the template-arguments (unique_ptr<T>).

  6. You are missing implicit upcasting in the move-ctor and move-assignment-operator.

    template <class U, class = std::enable_if_t<
        std::has_virtual_destructor<T>() && std::is_convertible<U*, T*>()>>
    unique_ptr(unique_ptr<U>&& other) noexcept
    : ptr_(other.release())
    {}
    
    template <class U>
    auto operator=(std::unique_ptr<U>&& other) noexcept
    -> decltype((*this = unique_ptr(other))) {
        return *this = unique_ptr(other);
    }
    
  7. Take a look at std::exchange(object, value) from <utility>. It allows you to simplify some of your code.

  8. If you use move-and-swap, you could isolate freeing of the referee to the dtor. Having it at only one place ensures you always do it the same, and is a good first step for retrofitting custom deleters. Not to mention that it in many cases simplifies the implementation.

  9. (ptr != nullptr) can be simplified to ptr. In contexts where you have to force the type, !!ptr.

  10. Why are op==() and op!=() inline-friend-functions, but swap() isn't? That's inconsistent. It's especially puzzling as they are all written to use the public interface only.

  11. There is exactly one place where you don't have a single empty line between two functions, but two. Yes, that's nothing big.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.