Thread: Deleting an object if the constructor failed.

RAII.

Originally Posted by g4j31a5

@Sebastiani & CornedBee: RAII? what's that? sorry that's still new to my ear. I swear, the profs have never mentioned that word when I was in college. -_-a

@abachler: LPVOID is the same as void*, right? Although it's doable, it's too much of a hassle IMHO. I rarely use void* so that the code can be read better. :P

You know, I've also never used auto_ptr. It's not portable IIRC. Maybe I'll try it in the future. As for passing pointers out of scope, I usually does that and control them strictly by myself. So I have to make sure if the object is still alive or not before doing anything to it on different objects. I usually code what came up first in my head. I confess, I suck at designing my codes. :P

#include <memory>

using std::auto_ptr;

Sorry for the late reply...

@laserlight: you know, I never consider STL to be a standard library. I guess I was wrong.

@CornedBee: OIC. Well, I think that's what I usually do. I always think that the object that created another object should be the one deleting it. Didn't know it had a name. :P

I don't get my almamater. I guess Informatics is different with Computer Science. There are only 2 programming subjects, "Programming Language" where we were taught the basics with the modular C programming using Turbo C, and "Advanced Programming Language" where we were taught OOP, MFC, and COM programming all in one subject, a little bit of everything. Oh yeah, there is also "Algorithm and Data Structures". The professors have never even mention C++ Faq Lite or even Bjarne Stroustrup's book (the Holy Bible of us C++ programmers) yet they go thoroughly with Data Mining / Warehousing, DFD, UML, etc. We don't even have a copy of Stroustrup's book in the library. Or is it something that you should learn autodidactly?

@IceDane: err... I actually know about auto_ptr but has never used it. I don't feel secure with using something that automatically deletes itself like auto_ptr and any garbage collector.

I don't feel secure with using something that automatically deletes itself like auto_ptr and any garbage collector.

auto_ptr doesn't delete itself. It deletes the objects it manages. It's a very simple management strategy that says, "When I die, I take my pointee with me. Unless you take it away first."
*Not* using RAII objects like auto_ptr to manage memory is what I don't feel secure with. I completely rely on the type system to document and enforce my ownership assumptions.

>> Well, I think that's what I usually do. I always think that the object that created another object should be the one deleting it. Didn't know it had a name. :P

More specifically, anything allocated by new should be immediately assigned to an object that can properly manage the memory. And note that it must be able to do so not just within it's destructor but in assignment, etc.

>> I actually know about auto_ptr but has never used it. I don't feel secure with using something that automatically deletes itself like auto_ptr and any garbage collector.

That's basically saying that you aren't comfortable with the idea of using RAII. In fact, you should convince yourself that it's the only way to go!

Anyway, just keep in mind that std::auto_ptr is a 'greedy' manager - if assigned to another std::auto_ptr it will take over ownership of the data. Here's a more friendly variant:

Code:

/*
	A simple memory manager.
	Note: 
	DO NOT use this to manage dynamically-allocated 
	arrays (use std::vector or similar for that)
*/
template < typename Type > 
class managed 
{
	public:
 
	managed( Type* ptr = 0 )
	: m_ptr_( ptr ) 
	{	}

 	managed( managed const& rhs )
	: m_ptr_( 0 ) 
	{	
		reset( rhs );
	}
	
	managed& reset( Type* ptr = 0 )
	{
		delete m_ptr_;
		m_ptr_ = ptr;
		return *this;
	}
 
	inline managed& operator = ( Type* ptr )
	{
		return reset( ptr );
	}
 
 	inline managed& reset( managed const& rhs )
	{
		return reset( new Type( *rhs ) );
	}

	inline managed& operator = ( managed const& rhs )
	{	
		return reset( rhs );
	}
	
	inline operator Type* ( void )
	{
		return m_ptr_;
	}

	inline operator Type const* ( void ) const
	{
		return m_ptr_;
	}
	
	inline Type* operator -> ( void )
	{
		return m_ptr_;
	}

	inline Type const* operator -> ( void ) const
	{
		return m_ptr_;
	}
	
	virtual ~managed( void ) 
	{
		reset( );
	}
 
	protected:

	Type* m_ptr_;
};

// Example usage
	
#include <iostream>

class test
{
	public:
	
	test( void )
	{
		std::cout << "test( )" << std::endl;	
	}

	test( test const& )
	{
		std::cout << "test( test const& )" << std::endl;	
	}
	
	virtual ~test( void )
	{
		std::cout << "~test( )" << std::endl;	
	}	
};

int main( void )
{
	managed< test >
		mt1 = new test( ), 
		mt2( mt1 ), 
		mt3( mt2 );
/*
	Fine, memory managed by 'mt3' is cleaned up in assignment
*/
	mt3 = mt1;
/*
	Fine, all memory is cleaned up when objects go out of scope
*/
	return 0;	
}

Code:

	inline operator Type* ( void )
	{
		return m_ptr_;
	}

	inline operator Type const* ( void ) const
	{
		return m_ptr_;
	}
	
	inline Type* operator -> ( void )
	{
		return m_ptr_;
	}

	inline Type const* operator -> ( void ) const
	{
		return m_ptr_;
	}

The first two are better implemented as operator*

Code:

inline Type& operator*() { return *m_ptr_; }
inline const Type& operator*() const { return *m_ptr_; }

If you want to give user access to the pointer itself, provide a get method.

I also suspect it might need quite a bit more to be truly useful (think about storing pointers to polymorphic objects). And if you can always make a copy of the object, why use a pointer and dynamic allocation in the first place?

Hmm, this managed() has incorrect behavior for polymorphic objects, and an implicit conversion to the underlying pointer type. Dangerous.
Not that auto_ptr isn't dangerous, especially on MSVC 2005, where there's a very, very nasty bug.

You really want a nice unique_ptr implementation. Experimental C++0x compilers provide one (GCC 4.3+, MSVC 10). There's also an emulated C++03 version floating around on the Boost mailing list.

There's no reason why this would not be compatible for polymorphic types unless the base class is purely abtract. As long as a fully constructed object of the base type is in a stable state, everything works fine. In that respect, it might even just serve as a placeholder that simply throws an exception if any actual methods are invoked. At any rate, what you're describing is exactly what std::auto_ptr was designed for - to manage objects that cannot be copy-constructed in their templated form.

It just causes object splicing on copy. If that is what I wanted, I wouldn't even bother to allocate anything dynamically.

Code:

Base b = Derived();

This is just as useful as your smart pointer (for polymorphic types).

For other types it would seem that you get the overhead of both dynamic allocation and copying. (And objects too large to fit onto stack (?) would probably be noncopyable anyway.)

Not that auto_ptr isn't dangerous, especially on MSVC 2005, where there's a very, very nasty bug.

...and it's copy semantics make it less of a smart pointer than one would think.

If you really want a great treatment of smart pointers I would go with boost. It works and it's simple to 'boost-ize' existing code which makes it a win win situation in my book. The only downside to using boost is that the MSVC debugger is often too stupid to peer into templated code and it will cause Intellisense to sit there scratching its head. Sometimes Intellisense works on boost or templates in general but most of the time it doesn't have a clue. Clearly this is a problem with the debugger and Intellisense and not boost so I cannot really blame boost for the issues. I've been using boost a lot in my code now and am beginning to wonder how I ever survived without it. Boost is far more than just a smart pointer library which also is a big plus. Boost is huge and I'm just beginning to explore some of it a bit more. I highly recommend it.

>> It just causes object splicing on copy. If that is what I wanted, I wouldn't even bother to allocate anything dynamically.

Well, in that case you'd need a smarter (and, of course, more 'expensive') pointer. I didn't test this much, but it should be pretty close:

Code:

/*
	A simple memory manager. 
	Note:
	- This class will not prevent 'slicing' on copy.
	- DO NOT use this to manage dynamically-allocated 
	arrays (use std::vector or similar for that)
*/
namespace shallow {

template < typename Type > 
class managed 
{
	public:
 
	managed( Type* ptr = 0 )
	: m_ptr_( ptr ) 
	{	}

 	managed( managed const& rhs )
	: m_ptr_( 0 ) 
	{	
		reset( rhs );
	}
	
	managed& reset( Type* ptr = 0 )
	{
		delete m_ptr_;
		m_ptr_ = ptr;
		return *this;
	}
 
	inline managed& operator = ( Type* ptr )
	{
		return reset( ptr );
	}
 
 	inline managed& reset( managed const& rhs )
	{
		return reset( new Type( *rhs ) );
	}

	inline managed& operator = ( managed const& rhs )
	{	
		return reset( rhs );
	}
	
	inline operator Type* ( void )
	{
		return m_ptr_;
	}

	inline operator Type const* ( void ) const
	{
		return m_ptr_;
	}
	
	inline Type* operator -> ( void )
	{
		return m_ptr_;
	}

	inline Type const* operator -> ( void ) const
	{
		return m_ptr_;
	}
	
	virtual ~managed( void ) 
	{
		reset( );
	}
 
	protected:

	Type* m_ptr_;
};

} // namespace shallow

/*
	A simple memory manager. 
	Note: 
	DO NOT use this to manage dynamically-allocated 
	arrays (use std::vector or similar for that)
*/
template < typename Type > 
class managed 
{
	protected:
	
	struct generator_base
	{
		virtual Type* clone( Type const* ) const = 0;
		
		virtual generator_base* clone( void ) const = 0;
	};
	
	template < typename Derived >
	struct generator : generator_base
	{
		virtual Type* clone( Type const* ptr ) const
		{
			return ptr ? new Derived
			( 
				*static_cast< Derived const* >( ptr ) 
			) 
			: 0;
		}
		
		virtual generator_base* clone( void ) const
		{
			return new generator( );
		}		
	};

	public:

	managed( Type* ptr = 0 )
	{	
		reset( ptr );
	}

	template < typename Derived >
	managed( Derived* ptr )
	{	
		reset( ptr );
	}

 	managed( managed const& rhs )
	{	
		reset( rhs );
	}

	inline managed& reset( Type* ptr = 0 )
	{
		return reset< Type >( ptr );
	}
	
	template < typename Derived >
	managed& reset( Derived* ptr )
	{
		m_ptr_ = ptr;
		m_gen_ = new generator< Derived >( );
		return *this;
	}

	template < typename Derived >
	inline managed& operator = ( Derived* ptr )
	{
		return reset( ptr );
	}
 
 	inline managed& operator = ( Type* ptr )
	{
		return reset< Type >( ptr );
	}
	
 	managed& reset( managed const& rhs )
	{
		m_ptr_ = rhs.m_gen_->clone( rhs.m_ptr_ );
		m_gen_ = rhs.m_gen_->clone( );
		return *this;
	}

	inline managed& operator = ( managed const& rhs )
	{	
		return reset( rhs );
	}
	
	inline operator Type* ( void )
	{
		return m_ptr_;
	}

	inline operator Type const* ( void ) const
	{
		return m_ptr_;
	}
	
	inline Type* operator -> ( void )
	{
		return m_ptr_;
	}

	inline Type const* operator -> ( void ) const
	{
		return m_ptr_;
	}
 
	protected:

	shallow::managed< generator_base >
		m_gen_;
	shallow::managed< Type >
		m_ptr_;
};

// Example usage
	
#include <iostream>

struct base
{
	virtual void test( void )
	{
		std::cout << "base::test( )" << std::endl;	
	}
};

struct derived : base
{
	virtual void test( void )
	{
		std::cout << "derived::test( )" << std::endl;	
	}
};

int main( void )
{
	managed< base >
		mb1 = new derived( ),
/*
	Fine, pointer to a derived object is constructed
*/		
		mb2( mb1 );
	mb1->test( );
	mb2->test( );
	return 0;	
}

EDIT: Fixed minor bug.

EDIT#2: Added minor functionality.

EDIT#3: Streamlined some things.

EDIT#4: Minor bug-fix.

Cool, well I've figured out a much simpler design that eliminates the extra allocation altogether. What do you guys think?

Code:

/*
	A simple memory manager. 
	Note: 
	DO NOT use this to manage dynamically-allocated 
	arrays (use std::vector or similar for that)
*/
template < typename Type > 
class managed 
{
	protected:
	
	struct generator_base
	{
		virtual Type* clone( Type const* ) const = 0;
	};
	
	template < typename Derived >
	struct generator : generator_base
	{
		virtual Type* clone( Type const* ptr ) const
		{
			return ptr ? new Derived
			( 
				*static_cast< Derived const* >( ptr ) 
			) 
			: 0;
		}
		
		static generator
			global;
	};

	public:

	managed( Type* ptr = 0 )
	: m_ptr_( 0 )
	{	
		reset( ptr );
	}

	template < typename Derived >
	managed( Derived* ptr )
	: m_ptr_( 0 )
	{	
		reset( ptr );
	}

 	managed( managed const& rhs )
	: m_ptr_( 0 )
	{	
		reset( rhs );
	}

	inline managed& reset( Type* ptr = 0 )
	{
		return reset< Type >( ptr );
	}
	
	template < typename Derived >
	inline managed& reset( Derived* ptr )
	{
		internal_reset( ptr );
		m_gen_ = &generator< Derived >::global;
		return *this;
	}

	template < typename Derived >
	inline managed& operator = ( Derived* ptr )
	{
		return reset( ptr );
	}
 
 	inline managed& operator = ( Type* ptr )
	{
		return reset< Type >( ptr );
	}
	
 	inline managed& reset( managed const& rhs )
	{		
		internal_reset( rhs.m_gen_->clone( rhs.m_ptr_ ) );		
		m_gen_ = rhs.m_gen_;
		return *this;
	}

	inline managed& operator = ( managed const& rhs )
	{	
		return reset( rhs );
	}
	
	inline operator Type* ( void )
	{
		return m_ptr_;
	}

	inline operator Type const* ( void ) const
	{
		return m_ptr_;
	}
	
	inline Type* operator -> ( void )
	{
		return m_ptr_;
	}

	inline Type const* operator -> ( void ) const
	{
		return m_ptr_;
	}

	inline friend bool operator == ( managed< Type > const& lhs, Type const* rhs )
	{
		return lhs.m_ptr_ == rhs;
	}

	inline friend bool operator == ( Type const* lhs, managed< Type > const& rhs )
	{
		return rhs.m_ptr_ == lhs;
	}

	inline friend bool operator != ( managed< Type > const& lhs, Type const* rhs )
	{
		return lhs.m_ptr_ != rhs;
	}

	inline friend bool operator != ( Type const* lhs, managed< Type > const& rhs )
	{
		return rhs.m_ptr_ != lhs;
	}
	
	virtual ~managed( void )
	{
		reset( );
	}
 
	protected:
	
	void internal_reset( Type* ptr )
	{
	/*
		The temporary may be a bit overkill, but it ensures 
		that if the 'current' data's destructor throws an 
		exception, the 'new' memory will still be cleaned up.
	*/	
		Type*
			saved = m_ptr_;
		m_ptr_ = ptr;
		delete saved;
	}

	Type*
		m_ptr_;
	generator_base*
		m_gen_;
};

template < typename Type > 
template < typename Derived > 
managed< Type >::generator< Derived >
	managed< Type >::generator< Derived >::global 
	= managed< Type >::generator< Derived >( ); 

// Example usage
	
#include <iostream>

struct base
{
	virtual void test( void )
	{
		std::cout << "base::test( )" << std::endl;	
	}
};

struct derived : base
{
	virtual void test( void )
	{
		std::cout << "derived::test( )" << std::endl;	
	}
};

int main( void )
{
	managed< base >
		mb1 = new derived( ),
/*
	Fine, pointer to a derived object is constructed
*/		
		mb2( mb1 );
	mb1->test( );
	mb2->test( );
	return 0;	
}

EDIT: Fixed some really silly bugs.

EDIT#2: Minor restructure

EDIT#3: Added comparison operators

Originally Posted by g4j31a5

@anon: OIC, I thought we can throw anything at anytime. My mistake then. BTW, your code didn;t work. I tried this one though and it worked:

Code:

B* A:CreateB(std::string paramStr)
{
    B* pB= NULL;
    try
    {
         pB = new pB (paramStr);
    }
    catch (int result)
    {
          pB = NULL;
    }

    m_VectorB.push_back(pB);
    return pB;
}

B::B(std::string paramStr)
{
    int result = initMembers (paramStr); //returns OK / 1 if successful or FAILED / 0 if failed
    if (result == FAILED)
                   throw result ;
}

It's not that anon's code "didn't work". It's that his code propagates the exception to the caller of CreateB rather than catching it internally and returning NULL, thus emmulating the behaviour of new (which is a good thing). Your code instead more closely emmulates the behviour of new no-throw. However it also has what is probably a bug, in that it pushes NULL onto the vector!
You also don't need to set pB to NULL in the catch block. If the new statement threw an exception then it hasn't gotten around to assigning anything to pB, thus it will still be NULL.

It is rather cool. Wouldn't the use of a static variable mean though, that this class is automatically not thread-safe, even if I don't visibly share any managed instance between threads?

Now it only remains to find a good use case for it. Treating polymorphic objects as value types?