snippetcppMinor
I wrote a class to implement auto_ptr
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
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
The destructor
Why the sudden use of this!
Assignment not exception safe.
The correct way to do this.
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.