Skip to main content
3 of 8
deleted 2 characters in body

Review

The code is well formatted and also reasonably well implemented, lacking only a few security overhauls such as memory leaks and optimizations also completing the pending implementations of existing concepts in unique_ptr

Is it well-formed?

Yes, have good formatting

i.e. does it follow common C++ standard and patterns (for example, should private members be declared before public ones?

Personally I think so.

  • Private Variables
  • Public
    • Constuctor / Destructor
    • Copy Semantics
    • Move Semantics
    • Swap
    • Other Public Interface
    • Friends
  • Private
    • Methods as appropriate

When reading the code I want to know the members so I can verify that the constructors initialize them all, as a result I usually put them first. But other people prefer to put all private stuff at the bottom.

Am I missing something regarding functionality?

Yes, you forgot to add T, and T[], and the following features:

type* release();

void reset(type* item);

void swap(unique_ptr &other);

type* get();

operator*;

operator->;

operator[];

Is there maybe a bug in my code that I'm not seeing?

Yes, you must force the receiving of types strictly to be pointers

Code Review

Copy assignment operator

unique_ptr<T>& operator=(const unique_ptr<T>& uptr) = delete;

This is redundant, this part is not necessary as this operator will never be called taking into account that the motion constructor exists and the copy constructor is disabled (= delete).

Template only typename T

template<typename T>
class unique_ptr {
   ...
}

You must accept both types T, and T[], ie: array or not.

Constructing from object

unique_ptr(T& t) {
    _ptr = &t;
}

That's exceedingly dangerous:

int x;
unique_ptr<int>  data(x);
//The unique ptr calls delete on an automatic variable.

NoExcept

unique_ptr(unique_ptr<T>&& uptr) noexcept
                               //^^^^^^^^
    
unique_ptr<T>& operator=(unique_ptr<T>&& uptr) noexcept
                                              //^^^^^^^^

The move operators should be marked as noexcept.

When used with standard containers this will enable certain optimizations. This is because if the move is noexcept then certain operations can be guaranteed to work and thus provide the strong exception guarantee.

Verify in move assignment operator

unique_ptr<T>& operator=(unique_ptr<T>&& uptr) {
   if (this == uptr) return *this;
   _ptr = std::move(uptr._ptr);
   uptr._ptr = nullptr;
   return *this;
}

Before _ptr =, you need to check if this->ptr is initialized, if yes delete ptr before assign.

Note: std::move in uptr._ptr is irrelevant.

Example code

I'll leave the complete code below based on your initial reasoning, I've added the outstanding parts and fixed the compatibility issue
#include <iostream>

using namespace std;

template<typename T>
class base_ptr {
private:
    using type = remove_extent_t<T>;
    virtual void finish() = 0;

protected:
    type* ptr = nullptr;

public:
    base_ptr(){};

    base_ptr(const base_ptr<T>& other) = delete;

    base_ptr(base_ptr<T>&& other) noexcept
    : ptr(other.ptr) {
       other.ptr = nullptr;
    }

    type* release(){
       type* tmp = ptr;
       ptr = nullptr;
       return tmp;
    }

    void reset(){
       finish();
    }

    void reset(type* item) noexcept {
       finish();

       ptr = item;
    }

    void swap(base_ptr<T> &other) noexcept {
       type* tmp = ptr;

       ptr = other.ptr;
      
       other.ptr = tmp;
    }

    operator bool(){
       return ptr;
    }

    type* get(){
        return ptr;
    }

    base_ptr<T>& operator=(base_ptr<T>&& other) noexcept {
       if (this == &other) return *this;
      
       finish();
       ptr = other.ptr;
       other.ptr = nullptr;
      
       return *this;
    }
};

template <typename T>
class unique_ptr : public base_ptr<T> {
protected:
    void finish() noexcept {
       if(base_ptr<T>::ptr != nullptr)
          delete base_ptr<T>::ptr;
    }

public:
    unique_ptr(){}

    unique_ptr(unique_ptr<T>&& other) 
    : base_ptr<T>(move(other)) { }

    explicit unique_ptr(T *ptr){
       base_ptr<T>::ptr = ptr;
    }

    unique_ptr<T>& operator=(unique_ptr<T>&& other){
       base_ptr<T>::operator=( move(other) );

       return *this;
    }

    T* operator->() const {
       return base_ptr<T>::ptr;
    }

    T& operator*() const {
       return *base_ptr<T>::ptr;
    }

    ~unique_ptr(){
       finish();
    }
};

template <typename T>
class unique_ptr<T[]> : public base_ptr<T[]> {
protected:
    void finish() noexcept {
       if(base_ptr<T[]>::ptr != nullptr)
          delete[] base_ptr<T[]>::ptr;
    }

public:
    unique_ptr(){}

    unique_ptr(unique_ptr<T[]>&& other) 
    : base_ptr<T[]>(move(other)) { }

    explicit unique_ptr(T *ptr){
       base_ptr<T[]>::ptr = ptr;
    }

    unique_ptr<T[]>& operator=(unique_ptr<T[]>&& other){
       base_ptr<T[]>::operator=( move(other) );

       return *this;
    }

    T& operator[](int pos) const {
       return base_ptr<T>::ptr[pos];
    }

    ~unique_ptr(){
       finish();
    }
};

template <class T, class ...Args>
enable_if_t<!is_array_v<T>, unique_ptr<T>>
make_unique(Args&& ...args)
{
   return unique_ptr<T>(new T(forward<Args>(args)...));
};

template <class T>
enable_if_t<is_array_v<T>, unique_ptr<T>>
make_unique(int size)
{
   using type = remove_extent_t<T>;
   return unique_ptr<T>(new type[size]);
};


int main() {
    unique_ptr<int> a (new int(1));
  
    unique_ptr<int> b;

    b.swap(a);

    b = make_unique<int>(2);
  
    cout << "End" << endl;
  
    return 0;
}