C++ pointer to class, call function where class-instance is freed, is that good practice?

Question

I have a class, let's call it A. There are two subclasses of class A which are a and b.

I'm making a pointer of class A like this:

A *pointer;

At some point in the program I initialize the pointer like this:

pointer = new a();

At some other point, I run a function of class A:

pointer->function(&pointer);

This function is inside class A (so all subclasses have it). There is a chance that when this function is called, I want to change pointer to another subclass, here is what I tried:

void A::function(A **pointer)
{
    if (something)
    {
        delete *pointer;
        *pointer = new b();
    }
}

Although this works, I'm really curious if this is good practice, I'm calling delete from inside the object and freeing the object itself, could this be undefined behavior and I got lucky it worked? Am I not understanding this right? Am I making this more complicated than it should be?

Answer 1

Yes, that's valid as long as you are careful. See more discussion at a question specifically about delete this.

However, as with other things in C++ which are valid as long as you are careful, you are better to find another solution that will be less prone to errors. I suggest you reworking you code into a function returning a new pointer, and having the old one automatically destroyed (via a smart pointer, for example).

Something along the lines:

struct A {
    static std::shared_ptr<A> function(std::shared_ptr<A>& ptr, int x) {
        if (x > 0)
            return std::make_shared<A>(x);
        else return ptr;
    }

    A(int _x): x(_x) {}

    int x;
};

Note also I made function() to be static, as it anyway accepts the object as its first argument. See live on coliru.

In fact, I don't quite like this solution with shared_ptr, and if someone will give a better implementation of this approach, I'll be glad to know.

Answer 2

This code is valid ( for more information about correctness see this answer ).
But it's not a good practice, because other developer can miss nuance, that using one of member functions will lead to reconstruction of object.
It's better to explicitly reconstruct object than hide it in member function. Or just use smart pointers.