I know the example is simplified, but it is too simple to show the issue. When you say the destructor nulls ALL members, I don't know if you are confused (there is no need for a destructor to set a member to null) or using different terminology (maybe your destructor calls delete to free memory and destroy objects).
If you look above at my second example, there is a call to new in the constructor. Is that what happens in your code (or something similar)? Here is a more detailed (and complete) example:
Code:
#include <iostream>
class DialogException { };
class DialogBox
{
int* member1;
int* member2;
public:
DialogBox(int argument1, int argument2)
{
if (argument1 < 0)
{
// ERROR! Should not be less than 0.
throw DialogException();
}
member1 = new int(argument1); // will be leaked
if (argument2 < 0)
{
// ERROR! Should not be less than 0.
throw DialogException();
}
member2 = new int(argument2);
}
~DialogBox()
{
std::cout << "In destructor.\n"; // never called
delete member1;
delete member2;
}
};
int main()
{
try
{
DialogBox dlg(1, -1);
}
catch (const DialogException& ex)
{
std::cout << "error initializing dialog.\n";
}
}
The problem there is that member1 will get leaked when argument2 is less than 0. Is this the kind of problem you are talking about?
To fix this, you have several options:
First, if possible, perform all tasks that might indicate a failed construction before you allocate anything that needs to be cleaned up. In this example, you can check argument2 before allocating space for member1. If you don't call new for member1 before you check argument2, then you won't leak that memory.
Sometimes that's not possible, so the second option is to have each member cleanup after itself. This is good design practice anyway, and for just this reason. If you use a smart pointer instead of a raw pointer, then the smart pointer will automatically cleanup when the exception is thrown:
Code:
#include <iostream>
#include <memory>
class DialogException { };
class DialogBox
{
std::auto_ptr<int> member1;
std::auto_ptr<int> member2;
public:
DialogBox(int argument1, int argument2)
{
if (argument1 < 0)
{
// ERROR! Should not be less than 0.
throw DialogException();
}
member1.reset(new int(argument1)); // will not be leaked
if (argument2 < 0)
{
// ERROR! Should not be less than 0.
throw DialogException();
}
member2.reset(new int(argument2));
}
// cleanup is automatic, no need for destructor.
// ~DialogBox()
private:
// disable copying.
DialogBox(const DialogBox&);
void operator=(const DialogBox&);
};
int main()
{
try
{
DialogBox dlg(1, -1);
}
catch (const DialogException& ex)
{
std::cout << "error initializing dialog.\n";
}
}
Note that auto_ptr might not be the best choice for a smart pointer, since it has strange copy semantics, but you probably won't be copying dialogs anyway so you can disable copying and be happy.
A third option, which is the most clumsy and error prone is to check for the error state and cleanup as you go. You can do this before throwing the exception:
Code:
#include <iostream>
class DialogException { };
class DialogBox
{
int* member1;
int* member2;
public:
DialogBox(int argument1, int argument2)
{
if (argument1 < 0)
{
// ERROR! Should not be less than 0.
throw DialogException();
}
member1 = new int(argument1);
if (argument2 < 0)
{
// Cleanup first member before throwing exception
delete member1;
throw DialogException();
}
member2 = new int(argument2);
}
~DialogBox()
{
std::cout << "In destructor.\n"; // never called
delete member1;
delete member2;
}
};
int main()
{
try
{
DialogBox dlg(1, -1);
}
catch (const DialogException& ex)
{
std::cout << "error initializing dialog.\n";
}
}
Hopefully those examples make sense and generally apply to your situation. I like the second option best.
