Skip to main content
1 of 3
Toby Speight
  • 88.3k
  • 14
  • 104
  • 327

First impressions are that this is pretty good code. Most of my comments will be nit-picks.


We include a lot of irrelevant headers. The unique_ptr type definition needs only <cstddef> and <utility> (also <optional> for the comparison with nullopt_t, but that's not part of std::unique_ptr's interface, and should be removed).

The test program needs <cassert>, but that shouldn't be part of the unique_ptr definition.


Comments such as these add no insight:

    // Default c'tor 
    // Move assignment operator

These should just be removed.


operator=() looks more complex than it needs to be. It's normally implemented the same as move-assignment, rather than setting the moved-from value empty:

        unique_ptr& operator=(unique_ptr&& other)
        {
            std::swap(ptr, other.ptr);
            return(*this);
        }

The constructor that accepts a raw pointer should be explicit. Without that, we're setting ourselves up for trouble when it is selected as a conversion and then the short-lived unique-pointer deletes the value!


The get function and the *, -> and bool operators should all be const, since they must not modify the stored pointer.

Many member functions should be noexcept - it's worth re-reading the specification of std::unique_ptr to confirm.


delete_underlying_ptr should probably be private, since it's not part of the standard interface. Its test for non-null ptr is redundant, and can be omitted:

        void delete_underlying_ptr()
        {
            delete ptr;
            ptr = nullptr;
        }

This is so simple that I question whether it needs to be a function.


make_unique() needs to return a unique_ptr, not a raw pointer. As it stands, users will get a leak when they write perfectly reasonable code:

{
    auto const p = std::make_unique<>…();
    ⋮
}

The relational operators == and != should accept constant arguments. And comparison with nullptr_t should be symmetric: we need an overload with the nullptr_t as first argument.

These operators ought to be declared in the dev namespace - remember that argument-dependent lookup will find them.


The test program could be improved by using a test framework instead of assert() so that it continues to report failures rather than aborting on the first one (or perhaps ignoring all the tests, when NDEBUG is defined). Remember that assert() exists for documenting things known to be true, not for making comparisons!

The test for release() doesn't delete the released object, resulting in a Valgrind failure. That's easily fixed:

        double* rawPtr = ptr.release();
        ⋮
        delete rawPtr;

Modified code

As well as addressing the points above, the member functions and their signatures correspond exactly to those of std::unique_ptr so the type is a drop-in replacement, save for the known limitations of not supporting arrays or custom deleters.

#include <cstddef>
#include <utility>

namespace dev
{
    template<typename T>
    class unique_ptr
    {
        T* ptr;

    public:
        unique_ptr() noexcept
            : ptr{nullptr}
        {}

        explicit unique_ptr(T* p) noexcept
            : ptr{p}
        {}

        // This is a move-only type
        unique_ptr(const unique_ptr& u) = delete;
        unique_ptr(unique_ptr&& other) noexcept
            : ptr{nullptr}
        {
            std::swap(ptr, other.ptr);
        }

        unique_ptr& operator=(const unique_ptr&) = delete;
        unique_ptr& operator=(unique_ptr&& other) noexcept
        {
            std::swap(ptr, other.ptr);
            return *this;
        }
        unique_ptr& operator=(nullptr_t) noexcept
        {
            reset();
            return *this;
        }

        ~unique_ptr()
        {
            delete ptr;
        }

        // ----
        T& operator*() const noexcept
        {
            return *ptr;
        }

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

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

        // ----
        T* get() const noexcept
        {
            return ptr;
        }

        void reset(T* other) noexcept
        {
            if (ptr != other) {
                delete ptr;
            }
            ptr = other;
        }

        void reset(nullptr_t = nullptr) noexcept
        {
            delete ptr;
            ptr = nullptr;
        }

        T* release() noexcept
        {
            return std::exchange(ptr, nullptr);
        }

        void swap(dev::unique_ptr<T>& other) noexcept
        {
            std::swap(ptr, other.ptr);
        }
    };


    template<typename T, typename... Args>
    unique_ptr<T> make_unique(Args&&... args)
    {
        return unique_ptr{new T(std::forward<Args>(args)...)};
    }

    template<typename T>
    void swap(unique_ptr<T>& a, unique_ptr<T>& b) noexcept
    {
        a.swap(b);
    }


    template<typename T1, typename T2>
    bool operator==(const unique_ptr<T1>& lhs, const unique_ptr<T2>& rhs)
    {
        return lhs.get() == rhs.get();
    }

    template<typename T>
    bool operator==(const unique_ptr<T>& lhs, std::nullptr_t)
    {
        return lhs.get() == nullptr;
    }

    template<typename T>
    bool operator==(std::nullptr_t, const unique_ptr<T>& rhs)
    {
        return rhs.get() == nullptr;
    }

    //Overload operator!=
    template<typename T1, typename T2>
    bool operator!=(const unique_ptr<T1>& lhs, const unique_ptr<T2>& rhs)
    {
        return !(lhs == rhs);
    }

    template<typename T>
    bool operator!=(const unique_ptr<T>& lhs, std::nullptr_t)
    {
        return lhs.get() != nullptr;
    }

    template<typename T>
    bool operator!=(std::nullptr_t, const unique_ptr<T>& rhs)
    {
        return rhs.get() != nullptr;
    }

}
Toby Speight
  • 88.3k
  • 14
  • 104
  • 327