Let's say I have a class with several pointers that are allocated in the constructor:

[source]
class some_class{
public:
some_class(int);
~some_class();

private:
int *ptr1;
int *ptr2;
int *ptr3;
int *ptr4;
};
[/source]

In the constructor, I would do this:

[source]
some_class::some_class(int some_val)
{
try{
ptr1 = new int(some_val);
}
catch(bad_alloc)
{
std::cout << "Unable to allocate memory." << std::endl;
return; // Skip the rest of the constructor.
}
// Begin the absurdity...
try{
ptr2 = new int(some_val);
}
catch(bad_alloc)
{
delete ptr1; // We can safely assume that ptr1 was allocated by this point.
ptr1 = NULL; // Safe delete...
std::cout << "Unable to allocate memory." << std::endl;
return; // Skip the rest of the constructor.
}
try{
ptr3 = new int(some_val);
}
catch(bad_alloc)
{
delete ptr1; // We can safely assume that ptr1 was allocated by this point.
ptr1 = NULL; // Safe delete...
delete ptr2; // We can safely assume that ptr2 was allocated by this point.
ptr2 = NULL; // Safe delete...
std::cout << "Unable to allocate memory." << std::endl;
return; // Skip the rest of the constructor.
}
try{
ptr4 = new int(some_val);
}
catch(bad_alloc)
{
delete ptr1; // We can safely assume that ptr1 was allocated by this point.
ptr1 = NULL; // Safe delete...
delete ptr2; // We can safely assume that ptr2 was allocated by this point.
ptr2 = NULL; // Safe delete...
delete ptr3; // We can safely assume that ptr3 was allocated by this point.
ptr3 = NULL; // Safe delete...
std::cout << "Unable to allocate memory." << std::endl;
}
}
[/source]

As you can see, depending on the number of pointers to be allocated, this can approach absurd levels rather quickly. However...

[source]
some_class::some_class(int some_val)
{
try{
ptr1 = new int(some_val);
}
catch(bad_alloc)
{
std::cout << "Unable to allocate memory." << std::endl;
return; // Skip the rest of the constructor.
}
// Much more clarity, less absurdity...
try{
ptr2 = new int(some_val);
}
catch(bad_alloc)
{
some_class::~some_class();
std::cout << "Unable to allocate memory." << std::endl;
return; // Skip the rest of the constructor.
}
try{
ptr3 = new int(some_val);
}
catch(bad_alloc)
{
some_class::~some_class();
std::cout << "Unable to allocate memory." << std::endl;
return; // Skip the rest of the constructor.
}
try{
ptr4 = new int(some_val);
}
catch(bad_alloc)
{
some_class::~some_class();
std::cout << "Unable to allocate memory." << std::endl;
}
}

some_class::~some_class()
{
if(ptr1 != NULL)
{
delete ptr1;
ptr1 = NULL;
}
if(ptr2 != NULL)
{
delete ptr2;
ptr2 = NULL;
}
if(ptr3 != NULL)
{
delete ptr3;
ptr3 = NULL;
}
if(ptr4 != NULL)
{
delete ptr4;
ptr4 = NULL;
}
}
[/source]

This is much more clear and compiles, however, I have heard that calling destructors directly is either a bad thing or frowned upon. Am I doing this correctly in the first place? Is there a better way? Is it OK to call the destructor in this case?

Use a smart pointer class instead and let the compiler do all that job for you.

struct some_class {
some_class(int some_val) :
ptr1(new int(some_val)),
ptr2(new int(some_val)),
ptr3(new int(some_val)),
ptr4(new int(some_val))
{ }

std::unique_ptr<int> ptr1, ptr2, ptr3, ptr4;
};


True, but for the sake of argument...

Explicitly calling the destructor does not do what you think it does. In particular, calling the destructor of an object from within its constructor will not affect the members of the class, and you will still leak just as bad as before.

Interesting! OK, time to stop using C-style pointers...

BTW, what is this "safe delete" thing?

It comes from a book, "Teach yourself C++ in 24 hours." I don't remember the exact reason and no longer have the book, but something about calling delete on a NULL pointer is safe, but calling it on an uninitialized pointer can lead to problems. The book mentioned setting the pointer to NULL after delete in the event that delete is called twice (why this would happen, I do not know...). I have always done this.

OK, time to stop using C-style pointers...

More properly: "Time to stop defaulting to using C-style pointers to manually manage memory".
You can still use C-style pointers for non-memory management...
But you shouldn't manually manage memory yourself...
...unless you actually need to for performance reasons (which it sometimes is, even on normal projects).

It comes from a book, "Teach yourself C++ in 24 hours." I don't remember the exact reason and no longer have the book, but something about calling delete on a NULL pointer is safe, but calling it on an uninitialized pointer can lead to problems. The book mentioned setting the pointer to NULL after delete in the event that delete is called twice (why this would happen, I do not know...). I have always done this.
[/quote]
That's from someone trying to manually manage memory, and instead of fixing the problem (problem: delete gets called twice), pretends the problem doesn't exist by hiding it (delete won't delete a null pointer).

The most direct solution is: don't call delete twice on the same pointer.
A even better solution is: don't call delete (or new) at all*, let smart pointers do it for you.

*[size=2]See previous comment of, 'except when you actually need to'

[edit:] That's not entirely to say setting an invalid pointer to null is bad. Dereferencing a null pointer crashes your program, which is good! So if there is an opportunity to use a pointer after it's been deleted, set it to null... but you shouldn't actually be calling new or delete to manage memory. If I have a raw pointer (that isn't managing memory) that's pointing at something, null is the ideal value to assign to it when it's not valid, because dereferencing null guarantees to crash, while dereferencing random memory maybe might crash, and maybe might do something incredibly weird that won't show up for several weeks or months.
In some cases it actually even makes sense (usually to avoid unnecessary checks for cleaner code) to delete twice since it has no effect - but it's important to A) know the reason why you raw pointers are set to NULL, and B) not use it to "solve" a program crashing, but actually find out why the program is crashing.

By default:
- Prefer memory on the stack over dynamic memory.
- Prefer smart pointers over raw pointers when you actually need dynamic memory.
- Use raw pointers when you actually need performance in that one area.

Note: 'by default' does not mean 'always'. And in the same way, "prefer smart pointers" does not mean "never use raw pointers".

If you absolute want to avoid using smart pointers, you could try using a cleanup member function.


class some_class{
public:
some_class(int);
~some_class();

private:
void cleanup();

private:
int *ptr1;
int *ptr2;
int *ptr3;
int *ptr4;
};

some_class:some_class(int some_val)
: ptr1(nullptr), ptr2(nullptr), ptr3(nullptr), ptr4(nullptr)
{
try
{
ptr1 = new int(some_val);
ptr2 = new int(some_val);
ptr3 = new int(some_val);
ptr4 = new int(some_val);
}
catch (...)
{
cleanup();
throw;
}
}

some_class::~some_class()
{
cleanup();
}

void some_class::cleanup()
{
delete ptr4;
delete ptr3;
delete ptr2;
delete ptr1;
}

This takes advantage of the fact that it's OK to use the delete operator on a pointer equal to nullptr.

quite good suggestion, BUT: in the cleanup function check for NULL and if not, then delete and assign to NULL. . I know that delete checks if the pointer is NULL, but for teaching purposes it is good to suggest that. Also, setting it to NULL after deleting is not mandatory, but is, again, good practice and, perhaps, would keep the user to double delete the same pointer and/or access it after deletion.

Sorry to double post, but, for learning purposes, here's another suggestion: If you want to track "troublesome bugs" with memory allocation (i.e. using pointers after you delete them - happens more often than you think), you set them to an invalid, but easily recognizable value, kinda like 0xfefefefe. Then, when the program blows to bits, you look at the pointer in the debugger, and if it matches (or it is close) the 0xfefefefe, you know you have this problem. enjoy