HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcppMinor

I wrote a class to implement auto_ptr

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
auto_ptrwroteclassimplement

Problem

Help me review this code:

#include 
#include 
using namespace std;

template 
class auto_ptr
{
public:
    explicit auto_ptr(T* p = NULL):m_p(p){}
    auto_ptr(auto_ptr& other);
    auto_ptr& operator=(auto_ptr& other);
    T* get() const { return m_p; }
    void reset(T* p = NULL);
    T& operator * (){ return *m_p; }
    T* operator -> (){ return m_p; }
    ~auto_ptr()
    {
        if(m_p)
            delete m_p;
        m_p = NULL;
    }   
private:
    T* m_p;
    T* release();
};

int main()
{
    auto_ptr p(new int(3));
    cout p1(new int(10));
    p = p1;
    cout p2(new int(100));
    auto_ptrp3(p2);
    cout
T* auto_ptr::release()
{
    T* temp = this->m_p;
    this->m_p = NULL;
    return temp;
}

template 
auto_ptr::auto_ptr(auto_ptr& other)
{
    m_p = other.release();
}

template 
auto_ptr& auto_ptr::operator=(auto_ptr& other)
{   
    if(this == &other)
        return *this;
    delete m_p;
    m_p = other.release();
    return *this;
}

template 
void auto_ptr::reset(T* p)
{
    if(m_p != p)
    {
        delete m_p;
        m_p = p;    
    }
}

Solution

Assuming you are going to put this in a header file don't do thus

using namespace std;


See: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice

These should be const as they don't change the state of your auto_ptr

T& operator * (){ return *m_p; }
T* operator -> (){ return m_p; }


The destructor

~auto_ptr()
{
    if(m_p)            // No need to check for NULL
                       // delete on a NULL pointer is valid.
        delete m_p;

    m_p = NULL;        // This is pointless.
                       // The object is about to leave scope.
}


Why the sudden use of this!

template 
T* auto_ptr::release()
{
    T* temp = this->m_p;
          //  ^^^^^^
    this->m_p = NULL;
 // ^^^^^^
    return temp;
}


Assignment not exception safe.

template 
auto_ptr& auto_ptr::operator=(auto_ptr& other)
{   
    if(this == &other)
        return *this;

    // This is not exception safe.
    // T is completely unknown to you.
    // Therefore you must assume the person who wrote T is an idiot 
    // and therefore may throw an exception from there destructor.
    delete m_p;

    // If the above statement throws then your object is now in an undefined
    // state with m_p pointing at an invalid object.

    m_p = other.release();
    return *this;
}


The correct way to do this.

template 
auto_ptr& auto_ptr::operator=(auto_ptr& other)
{   
    if(this == &other)
        return *this;

    auto_ptr tmp = release();
    m_p = other.release();

    // After the state of your (and other objects)
    // are completely defined and are in a good state.
    // then you can tidy up any left over objects.

    return *this;

    // the destructor of tmp should now release it properly.
}

Code Snippets

using namespace std;
T& operator * (){ return *m_p; }
T* operator -> (){ return m_p; }
~auto_ptr()
{
    if(m_p)            // No need to check for NULL
                       // delete on a NULL pointer is valid.
        delete m_p;

    m_p = NULL;        // This is pointless.
                       // The object is about to leave scope.
}
template <typename T>
T* auto_ptr<T>::release()
{
    T* temp = this->m_p;
          //  ^^^^^^
    this->m_p = NULL;
 // ^^^^^^
    return temp;
}
template <typename T>
auto_ptr<T>& auto_ptr<T>::operator=(auto_ptr& other)
{   
    if(this == &other)
        return *this;

    // This is not exception safe.
    // T is completely unknown to you.
    // Therefore you must assume the person who wrote T is an idiot 
    // and therefore may throw an exception from there destructor.
    delete m_p;

    // If the above statement throws then your object is now in an undefined
    // state with m_p pointing at an invalid object.


    m_p = other.release();
    return *this;
}

Context

StackExchange Code Review Q#29734, answer score: 6

Revisions (0)

No revisions yet.