C++ Deleting during constructor
Author
I have a system where when u create a new Derived class the Base class registers the new Object with an object manager.
If the Object has invalid properties ( it has a string Id, if that Id is not unique in the entire system then the object is not valid ) the object should be deleted. Right now, the registration function is passed a pointer to the object in question. If found to be invalid it calls delete on the pointer. Then the Base class constructor finishes and the derived constructor executes and promptly crashes b/c it tried to use things from the base class which has now been deleted.
Obviously not a valid way of doing things. So here is the issue, things happen in the derived constructor that should not be allowed to happen if that registration() fails in the base constructor.
So I have 2 questions.
The first is, how can I prevent the derived constructors from either taken place or from executing the base class sensitive code. The only thing I can think of is setting some varriable like bool bValid; in the base class if the registration fails and checking that in the derived constructor.
My 2nd question is, if the registration fails, and the object should be deleted, and the delete is not carried out in any of the constructors since this does not seem like a very valid way of doing things. How should the instanciating programer know that the object is not valid and should be deleted?
I suppose if I was using that bValid varriable I could check that and delete depending on that, but that seems really crappy to me. I want to find a way to not have to do anything more then call the constructor, I would like new to simply return NULL if it was invalid.
Move the registering and checking outside of the constructor. Allocating or deleting memory in such a manner inside of a constructor is generally frowned upon, since you can run into the problems as mentioned above without the ability to notify the calling code of such an error. The constructor's main job is to insure that the object is properly initialized upon creation. Create a separate method, such as bool Register(), that is called to handle the registration routine after the object's successful creation. On failure of the registration, you can then safely destroy the object.
I have been wrestling with this kind of thing recently for a similar reason. I want to register a unique name when a subclass of my custom RTTI base class is instantiated, and I want it to be based on the actual class name. For example, if CSubClass extends CBaseClass, then the first instance of a CSubClass will be named CSubClass0 and the next will be CSubClass1, etc.
I want the actual naming logic to be in the CBaseClass constructor, because it is inconvenient and error prone to require every class derived from the base class to name itself. To get the class string, suppose I use a virtual function to get the name of the class, like so:
I make the default constructor protected and allow no other constructors so I know this will get called by subclasses' constructors and ensure the name is made. The problem is that the virtual function override for the subclass is not known at the time of the base class's constructor. This results in all objects being named "CBaseClassX", no matter the actual derived class.
I can conceive of two basic ways to solve this problem. One is to override the "new" operator or perhaps make the CBaseClass constructor private and only allow a static or friend function to construct a CBaseClass. Then I can put whatever code I want in to touch-up the object *after* the constructor has been called in this calling function. The second way is to register the object, which pretty much means adding it to a list of objects that have been created, which sounds similar to your approach. I can then have any methods that depend on derived construction evaluate lazily by checking whether or not the object is in the list. If it is, it initializes the sensitive data in the object. That way, as long as it isn't called in a derived constructor, it won't actually be set until it is ready.
The first way is more airtight, because there will be absolutely no way to construct an object derived from CBaseClass unless it is through the methods I choose. The problem is, it forces derived classes to use whichever constructor signatures I provide. If client subclasses want to define a new constructor with different inputs, they are stuck because they can't call the base class's constructor. The second way eliminates that problem but it opens up the door for user error by calling GetName or MakeName before the object has been fully constructed.
I want the actual naming logic to be in the CBaseClass constructor, because it is inconvenient and error prone to require every class derived from the base class to name itself. To get the class string, suppose I use a virtual function to get the name of the class, like so:
class CBaseClass{ protected: CBaseClass() { MakeName(GetName()); } private: void MakeName(char const *); // makes a unique name based on string public: virtual char const * GetName() const { return "CBaseClass"; }};class CSubClass : public CBaseClass{ virtual char const * GetName() const { return "CSubClass"; }};I make the default constructor protected and allow no other constructors so I know this will get called by subclasses' constructors and ensure the name is made. The problem is that the virtual function override for the subclass is not known at the time of the base class's constructor. This results in all objects being named "CBaseClassX", no matter the actual derived class.
I can conceive of two basic ways to solve this problem. One is to override the "new" operator or perhaps make the CBaseClass constructor private and only allow a static or friend function to construct a CBaseClass. Then I can put whatever code I want in to touch-up the object *after* the constructor has been called in this calling function. The second way is to register the object, which pretty much means adding it to a list of objects that have been created, which sounds similar to your approach. I can then have any methods that depend on derived construction evaluate lazily by checking whether or not the object is in the list. If it is, it initializes the sensitive data in the object. That way, as long as it isn't called in a derived constructor, it won't actually be set until it is ready.
The first way is more airtight, because there will be absolutely no way to construct an object derived from CBaseClass unless it is through the methods I choose. The problem is, it forces derived classes to use whichever constructor signatures I provide. If client subclasses want to define a new constructor with different inputs, they are stuck because they can't call the base class's constructor. The second way eliminates that problem but it opens up the door for user error by calling GetName or MakeName before the object has been fully constructed.
I would avoid doing that kind of external business in constructors. If you were to do this in an outside function it would be much easier. The managed object shouldn't have to know about its manager. Is it going to be calling member functions on its manager, telling it what to do? That seems backwards. Anyway, here is an example of what I'm talking about:
Now you can call that anywhere you would call new. I understand your creation process might be more complex, but like I said I think it should be handled in by an outside function. In most cases the constructor should only be responsible for setting up the internal state of the object (private data members, etc.). If you insist on having an Object know its Manager you can put a p->setManager(mgr) in there too.
Object* CreateObject(Manager& mgr){ Object* p = new Object(); if( !mgr.register(p) ) { delete p; p = NULL; } return p;}Now you can call that anywhere you would call new. I understand your creation process might be more complex, but like I said I think it should be handled in by an outside function. In most cases the constructor should only be responsible for setting up the internal state of the object (private data members, etc.). If you insist on having an Object know its Manager you can put a p->setManager(mgr) in there too.
Author
hhmmm ya I think some method involving a wrapper function for the constructor would be ok.
So that kinda works. Signals the creating code that it failed by returning NULL and any registration sensitive code can be put in the overridden newObject() function. If more arguments are required for other derived classes theres nothing that says they cant just create a new newObject() function.
This could probably be made more transparent by over riding new like you suggested but i've never done that so not sure how that would work.
Something about it doesn't sit with me well though... Plus every deriving class must do the registration thing it's self. I really want to keep that the responsibility of the Base class.
class Base {private:bool registerObj( Base *ptrBase );protected:virtual Base( int iSomeArg );public:Base * virtual newObject( int iSomeArg ) { Base* ptrBase = new Base( iSomeArg ); if( !registerObj( ptrBase ) ) { delete ptrBase; ptrBase = NULL; } return ptrBase;}};class Derived {protected:virtual Derived( int iSomeArg );public:Base * virtual newObject( int iSomeArg ) { Base* ptrBase = new Derived( iSomeArg ); if( !registerObj( ptrBase ) ) { delete ptrBase; ptrBase = NULL; } else { // Registration sensitive code here } return ptrBase;} };So that kinda works. Signals the creating code that it failed by returning NULL and any registration sensitive code can be put in the overridden newObject() function. If more arguments are required for other derived classes theres nothing that says they cant just create a new newObject() function.
This could probably be made more transparent by over riding new like you suggested but i've never done that so not sure how that would work.
Something about it doesn't sit with me well though... Plus every deriving class must do the registration thing it's self. I really want to keep that the responsibility of the Base class.
Encapsulation.
The expected behaviour of your object, from what I gather, is this:
I'll ignore for the time being the coupling problem in your design—the object is associated with a manager even though you never specified which manager it should be associated with, but this can make semantic sense in some situations.
Now, what you did not describe is what the user should expect when the identifier is already used by another object in the manager. Obviously, the code above idiomatically means that b will be a valid object, unless of course an exception occurs during memory allocation or object initialization. This is how new works and is expected to work in C++, so you have no reason or legitimity to circumvent this convention (it would both surprise users of your class, including yourself, and clash with the other conventions that were designed to interact correctly with it).
So, the usage that I would expect from your class (regardless of internal implementation) is:
This approach has several avantages:
So, to answer your questions:
The expected behaviour of your object, from what I gather, is this:
Base* b = new Derived("identifier");// here, b has an associated manager with which it is registeredI'll ignore for the time being the coupling problem in your design—the object is associated with a manager even though you never specified which manager it should be associated with, but this can make semantic sense in some situations.
Now, what you did not describe is what the user should expect when the identifier is already used by another object in the manager. Obviously, the code above idiomatically means that b will be a valid object, unless of course an exception occurs during memory allocation or object initialization. This is how new works and is expected to work in C++, so you have no reason or legitimity to circumvent this convention (it would both surprise users of your class, including yourself, and clash with the other conventions that were designed to interact correctly with it).
So, the usage that I would expect from your class (regardless of internal implementation) is:
try { Base* b = new Derived("identifier"); // here, b is valid} catch (const DuplicateIdException & e){ // the ID was already in use}This approach has several avantages:
- It works out-of the box. You don't have to wonder about undefined behaviour which might appear because of deletion during construction.
- It is clean: if your code is exception-safe (which it should), then the above will not leak memory (already constructed members and base classes will be destructed, and the allocated memory will be released, if any).
- It also works when the object is created without dynamic allocation, such as on the stack, as a global object, or in an SC++L container.
So, to answer your questions:
- You prevent derived constructors from executing by throwing an exception in the base constructor (or one of the functions it calls).
- You delete the object automatically by throwing an exception in the constructor (or one of the functions it calls), and the user can then catch the exception to be made aware of the construction failure.
The answer is to throw an exception instead of deleting it. If the constructor throws an exception, the memory is never allocated in the first place, and there is nothing to clean up.
I'm surprised it took so long for someone to bring this up.
I'm surprised it took so long for someone to bring this up.
Quote:Original post by Deyja
I'm surprised it took so long for someone to bring this up.
I think it's a general lack of confidence concerning exceptions. I was thinking of posting last night, but then realized that I don't use exceptions as often as I should (as in hardly ever), and thus am not confident on precisely how they should be used here, and what the results would be (partially constructed object would be properly destructed and deallocated), so I left it to someone else more competent. I wouldn't be surprised if there were a lot of other people who feel a similar way about exceptions.
That said, I think I should remedy this and keep exceptions continually in the back of my mind while working on my current project.
Quote:
I was thinking of posting last night, but then realized that I don't use exceptions as often as I should (as in hardly ever), and thus am not confident on precisely how they should be used here, and what the results would be (partially constructed object would be properly destructed and deallocated),
It's a good reservation to have, as in fact there are some caveats with respect to cleaning up properly in a constructor in the face of exceptions. A destructor for an object isn't called unless the object was fully-constructed, and an object is not fully-constructed until its constructor completes.
Consequently, when you throw from a constructor, the destructor matching it will not be called.
This means that in all but the most-trivial cases, you need to take action to ensure things get cleaned up. For example, if there are two memory allocations in your constructor, either one could fail -- if the second fails and an exception is thrown, the destructor would not be called and the first allocation would leak.
This is why we're given smart pointers and other methodologies for employing RAII, because they make handling this sort of thing much, much easier.
Author
Ya actually. The exception case does look like the cleanest method.
There is only ever one manager, and the Base class being the tightly coupled to the manager is the very point of the class, it wraps a lot of functionality of the manager for derived classed to have easy access to.
Sadly though I have designed this whole project with out using exceptions which I am regreting more and more as things go on. I am comfortable with exceptions in Java and C# but never used them in C++ im ashamed to admint.
I have a deeper inheretence tree then described though, soemthing like:
Base -> Sub1Class -> Sub2Class
So lets say Base finishes ok, but then something critical in Sub1Class fails and again the object should be deleted. Would throwing an exception there clean up the memory from Base? I wouldn't think so, so how would you handle that?
Lastly what you were saying about changing the behavior of new, inside the framework which all these derived classes will function, failing to register with the manager is as critical as failing to get the memory, so following the same scheme of deleting any allocated memory and returning NULL makes sense. Atleast in my mind. Does that seem really wrong to you guys?
There is only ever one manager, and the Base class being the tightly coupled to the manager is the very point of the class, it wraps a lot of functionality of the manager for derived classed to have easy access to.
Sadly though I have designed this whole project with out using exceptions which I am regreting more and more as things go on. I am comfortable with exceptions in Java and C# but never used them in C++ im ashamed to admint.
I have a deeper inheretence tree then described though, soemthing like:
Base -> Sub1Class -> Sub2Class
So lets say Base finishes ok, but then something critical in Sub1Class fails and again the object should be deleted. Would throwing an exception there clean up the memory from Base? I wouldn't think so, so how would you handle that?
Lastly what you were saying about changing the behavior of new, inside the framework which all these derived classes will function, failing to register with the manager is as critical as failing to get the memory, so following the same scheme of deleting any allocated memory and returning NULL makes sense. Atleast in my mind. Does that seem really wrong to you guys?
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
