Back to General and Gameplay Programming

What can I do when a error occurs in Object's constructor?

songge09 · 2010-07-05T05:21:49

I don't know how to handle the error in object's constructor in c++.Using exception?Or just write a function initialize() to replace the constructor? (I don't like the methodit is too c style)Please help me.

General and Gameplay Programming Programming

Started by songge09 June 30, 2010 03:04 AM

89 comments, last by phresnel 13 years, 9 months ago

Burnhard

June 30, 2010 05:29 PM

I don't see why Initialise should be the option of last resort. All classes I write that allocate resources, or that I create dynamically, follow the same pattern:

shared_ptr<ObjectType> myNewObject = CreateObjectType();

MyNewObject->Initialise(a, b, c, d, e, f);

Each class has an abstract interface in the header and an 'Impl in the .cpp file that derives from the abstract interface. Thus, for example:

// Abstract interface class Object{public:	virtual void Initialise(...) = 0; // THROWS};// Create methodshared_ptr<Object> CreateObject();

and in the .cpp:

class ObjectImpl:	public SharedBase<ObjectImpl>,	public Object{public:	ObjectImple() : A(0), B(0), C() {}	virtual void Initialise(...); // THROWSprotected:	// Not logically possible to copy or duplicate				ObjectImpl(const ObjectImpl&);	ObjectImpl& operator = (const ObjectImpl&);private:	A, B, C, etc...};... implementation ... followed by:shared_ptr<Object> CreateObject(){	return SharedObject<ObjectImpl>::Create();}

(SharedObject is a simple class that enforces non-copyable behavior and boost::enable_shared_from_this)

The advantage to me here is that I can modify the impl without having to re-compile everything that includes the abstract interface. By only ever initialising basic types in my constructor (assigning default values) and by using exceptions rather than return codes alongside smart pointers, I'm much better protected from the complexities of clean-up when things go wrong.

On the level of general principle then, I *never* allocate resources in a constructor and by using smart pointers I never have to implement a destructor either.

rip-off

11,000

June 30, 2010 05:43 PM

Quote:
I *never* allocate resources in a constructor and by using smart pointers I never have to implement a destructor either.

Unless your Base class implements a virtual destructor, you are likely leaking buckets of resources (you may have posted sample code - but I am saying this for the OP's benefit).

Given that Initialise can throw, what is the advantage of using a virtual Initialise over a constructor call?

E.g:

shared_ptr<Object> CreateObject(Args args...){	return boost::shared_ptr<Object>(new ObjectImpl(args...));}

blog | twitter

Burnhard

June 30, 2010 06:08 PM

I'm not leaving any resources. shared_ptr is designed to handle the case correctly, so virtual destructors are not necessary. You only need virtual destructors if you intend to delete a derived class instance through its base class. shared_ptr "knows" its original type, so this situation never occurs.

With respect to the advantage of Initialise over the constructor in terms of resource allocation, there are some instances, particularly with encapsulation, when you don't have all of the information or state you need in order to construct the object. In these cases you would need: (1) construct and (2) SetProperty(x), which is essentially no different from doing it all in an Initialise. I just make it explicit on all of my resource handling classes.

The only reason I can think of to do it all in the constructor is the worry that you have constructed an object that many not be usable. But if you put guards on your object methods to check for appropriate state (asserts perhaps) or in the run-time case throw, the mistake is pretty obvious and ceases to be a problem.

I'm not holding this up as the way to do things, it's just the way I've been taught to do them by guys at work with far larger foreheads than mine :p.

Edit: I should add that the destructor in the implementation is protected (and does nothing), to prevent the situation described at the top of this post from ever happening!).

SiS-Shadowman

359

July 01, 2010 04:27 AM

Quote:Original post by Burnhard
I'm not leaving any resources. shared_ptr is designed to handle the case correctly, so virtual destructors are not necessary. You only need virtual destructors if you intend to delete a derived class instance through its base class. shared_ptr "knows" its original type, so this situation never occurs.

I hope you realize that boost shared ptr does NOT know the original type. If you're not convinced then compile the following example and see that no text appears on the console. This is because you have NO virtual destructors.

struct Data{   ~Data() { std::cout << "I got destroyed" << std::endl;}struct A{};struct B : public class A{    Data d;};int main(int argc, const char* argv[]){    boost::shared_ptr<B> b(new B);    boost::shared_ptr<A> a = b;    b.swap(boost::shared_ptr<B>());}

Quote:
The only reason I can think of to do it all in the constructor is the worry that you have constructed an object that many not be usable. But if you put guards on your object methods to check for appropriate state (asserts perhaps) or in the run-time case throw, the mistake is pretty obvious and ceases to be a problem.

But why would I want to check the validity of my object in every fracking method of my class? If I worry about it in the constructor and throw if there's an error then I don't need to worry about anything else.

Gamedev-Journal

Burnhard

July 01, 2010 05:43 AM

"I hope you realize that boost shared ptr does NOT know the original type. If you're not convinced then compile the following example and see that no text appears on the console. This is because you have NO virtual destructors."

Dude, your example works. The destructor gets called (!). Try it yourself. I know I have no memory leaks because I've got a function that hooks into shared_ptr and checks for any existing allocations, just before program termination (giving me an exception in debug mode if anything remains allocated).

With respect to doing all your stuff in a constructor, as I said before, there are cases where you need (1) construct, (2) initialise/SetProperty(X), so why not make it an explicit principle on all of your stuff? Initialise guarantees an object is in a usable state. A constructor doesn't necessarily. And why are you not making your code more robust and errors easier to find by checking pre-conditions when your functions get called?

void
Foo:Add(int x)
{
THROWIFNULL(MyBar); // Not entirely unreasonable to check this, is it?

MyBar->Add(x);
}

SiS-Shadowman

359

July 01, 2010 06:14 AM

Quote:Original post by Burnhard
"I hope you realize that boost shared ptr does NOT know the original type. If you're not convinced then compile the following example and see that no text appears on the console. This is because you have NO virtual destructors."

Dude, your example works. The destructor gets called (!). Try it yourself. I know I have no memory leaks because I've got a function that hooks into shared_ptr and checks for any existing allocations, just before program termination (giving me an exception in debug mode if anything remains allocated).

Hm, that's what I get for not putting a breakpoint inside the destructor. You are right. Sorry for the garbage I posted.

Gamedev-Journal

Burnhard

July 01, 2010 06:17 AM

No problem! I had to ask a colleague because I wasn't sure and it isn't obvious.

phresnel

953

July 01, 2010 06:23 AM

Quote:Original post by Burnhard
"I hope you realize that boost shared ptr does NOT know the original type. If you're not convinced then compile the following example and see that no text appears on the console. This is because you have NO virtual destructors."

Dude, your example works. The destructor gets called (!). Try it yourself. I know I have no memory leaks because I've got a function that hooks into shared_ptr and checks for any existing allocations, just before program termination (giving me an exception in debug mode if anything remains allocated).

How does it hook into any existing shared_tpr<>?

Also, in case you have a shared_ptr<T>, and let it point to some U, where U is a subclass of T, then how exactly do you think will shared_ptr<> be able to save U?

The U-instance will only be properly destructed when shared_ptr<T> preserves a vtable-entry to U's destructor, but this is not given when you don't make the destructor virtual in T.

Or I completely missed some point.

edit: Tbh, don't really know what's up with this. But I keep my comment for public's sake :/

Quote:With respect to doing all your stuff in a constructor, as I said before, there are cases where you need (1) construct, (2) initialise/SetProperty(X), so why not make it an explicit principle on all of your stuff? Initialise guarantees an object is in a usable state. A constructor doesn't necessarily. And why are you not making your code more robust and errors easier to find by checking pre-conditions when your functions get called?

void
Foo:Add(int x)
{
THROWIFNULL(MyBar); // Not entirely unreasonable to check this, is it?

MyBar->Add(x);
}

Pre-condition is given if RAII instance exists. Band-aids do not make you more robust, they just make you stop bleeding.

[~/www/ — ~/projects/picogen — ~/projects/metatrace]

SiS-Shadowman

359

July 01, 2010 06:32 AM

Quote:Original post by Burnhard
With respect to doing all your stuff in a constructor, as I said before, there are cases where you need (1) construct, (2) initialise/SetProperty(X), so why not make it an explicit principle on all of your stuff? Initialise guarantees an object is in a usable state. A constructor doesn't necessarily. And why are you not making your code more robust and errors easier to find by checking pre-conditions when your functions get called?

One example where this is required is when you need to store a weak pointer to the object you've just created. I usually solve this by a static create function that creates the object, stores it in a shared_ptr and then sets the weak ptr as well.

class Foo{    static boost::shared_ptr<Foo> create()    {        boost::shared_ptr<Foo> foo(new Foo);        foo->m_weak = foo;        return foo;    }}

I didn't mean to express that it's a bad idea to separate them in general, but when it's required I like to make one call, ie. Object::create(....), which seems to be what you're talking about.

I think I misunderstood you on your last point as well: Ofcourse I do error checking in my functions, when it's necessary, but I never check if my objects are still in a valid state: instead I prevent my objects from entering them.

To the boost shared_ptr thing: This is awesome because you can share shared_ptrs across modules that link against different versions of the CRT without running into problems.

*edit*

Quote:Original post by phresnel
The U-instance will only be properly destructed when shared_ptr<T> preserves a vtable-entry to U's destructor, but this is not given when you don't make the destructor virtual in T.

Or I completely missed some point.

I stepped through their code and it seems shared_ptr is more sophisticated than storing a T*/U*.
They have a sp_counted_base that stores the reference counter AND an actual implementation: class sp_counted_impl_<T> : public sp_counted_base that preserves the original type.

Gamedev-Journal

Burnhard

July 01, 2010 06:55 AM

"How does it hook into any existing shared_tpr<>?"

Custom allocator/deallocator allows you to maintain collections of active objects, which you can walk just before shut-down to see if they're alive or not.

"Or I completely missed some point."

From what I can see looking at the call stack, shared_ptr<A> calls "release" on its encapsulated pi_ pointer (sp_counted_base) and release calls "dispose" on its encapsulated instance pointer (px_). Dispose is virtual, so it's called on B. The innards of boost *anything* is black magic to me however; I just use it because it works. Try the example and check the call stack from the destructor. If you can see what's going on I'd be interested to know :p.

With respect to bleeding, on general principle I prefer not to. When it comes to RAII, constructors and initialise, I use the following three principles:

(1) RAII where it makes sense to use it (for example, when I want exception safe locking semantics on resources)

(2) Constructor where it makes sense to use it (usually when the class is trivial)

(3) Initialise where it makes sense to use it (when I have to construct and then initialise (SetProperty...) in any case.

For (3), if you use constructs such as Boost::enable_shared_from_this, you can't access or use This() in the constructor, which can be a severe restriction if you need to send your "this" off to some other objects passed into the constructor. So instead of wrestling with the decision on a case-by-case basis, I just use it throughout: instance factory -> construct -> initialise.

What can I do when a error occurs in Object's constructor?

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

What can I do when a error occurs in Object's constructor?

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines