child class destructor is not being called despite of having virtual dest.

[can-mohan]

Hi All,
In below code i am surprised to see destructor for rectangle class is not being called though base class destructor for IShape is virtual.
below is the output of below programme:
draw the rectangle
draw the circle draw destructor
deleting circle

deleting the Rectangle is missing from output though constructor for Rectangle is called.
Is there any design flaw with this code ?

Note: in below code destruction of base class pointer is done through destructor of draw class.

 #include<iostream>
        #include <memory>
        using namespace std;
        typedef enum result
        {
         result_error = 0,
         result_success = 1
        } RESULT_T;
        class IShape
        {
        public:
        static IShape* create_instance(string param);
        RESULT_T draw ()
        {
         return Do_Draw();
        }
        virtual ~ IShape(){}
        private:
        RESULT_T virtual Do_Draw( )=0;

        };
        class Circle : public IShape
        {
         int* p;
         public:
         Circle()
         {
         p =new int (12);
         }

        RESULT_T Do_Draw()
        {
        cout<<"draw the circle ";
        return result_success ;
        }

         ~ Circle(){
        delete p;
         cout<<"deleting circle"<<endl;
        }

        };
        class Rectangle : public IShape
        {
        int *p;
        public:
        Rectangle()
        {
        p=new int (10);
        }

             RESULT_T Do_Draw()
        {
        cout<<"draw the rectangle "<<endl;
        return result_success ;
        }
        ~Rectangle(){

        delete p;
        cout<<"deleting the Rectangle"<<endl;
        }
        };
        IShape* IShape::create_instance(string param)
    {

    if(param=="Circle")
    {

    return new Circle;
    }

    else if (param=="Rectangle")
    {


    return new Rectangle;
    }

    return NULL;

    }

    class Draw
    {
    IShape *obj;
    public:
    void Set_draw(string param)
    {

    obj=(IShape::create_instance(param ));

    }

    void Draw_Shape()
    {

    obj->draw();
    }
    ~Draw( ){ cout<<"draw destructor"<<endl;delete obj;} // it is not responsible for deletion of IShape object.
    };



    int main()
    {
        RESULT_T final_ret_code=result_error;

        Draw obj;

         obj.Set_draw ("Rectangle");
         obj.Draw_Shape();
         obj.Set_draw ("Circle");
         obj.Draw_Shape();

        return 1;

    }

[AssertNull]

Is there any design flaw with this code ?

Yes. Line 90 creates a new object. obj in the Draw class is a pointer to an IShape object. If obj does not point to anything, it should be set as NULL. If it does point to a real IShape object, when you create a new IShape object and have obj point to that new IShape object, what happens to the IShape object that obj is no longer pointing to? It must be deleted somewhere if it is no longer in use. Otherwise you have a memory leak.

[can-mohan]

Thanks for your observation. It can be taken care by :

void Draw_Shape()
        {
        if(obj!=NULL)
        {
        obj->draw();
        }
     else
    {
    cout<<"deleting the object"<<endl;
    delete obj;
    }
    }

Now the question is why destructor is not being called for Rectangle object.

[AssertNull]

Thanks for your observation. It can be taken care by :

But it's not being taken care of there, is it? If it was, you would not have posted. And that code won't delete anything. delete obj is only called when obj is NULL.

Now the question is why destructor is not being called for Rectangle object.

I told you why the Rectangle destructor is not being called. You have a memory leak and it has nothing at all to do with the new code you posted above. Look at line 90. What if obj already points to an object? Let's pretend that obj points to a Rectangle at memory location 0x123456. You create a new Circle object at line 90. Let's say that new Circle object is at memory location 0x1234A0. obj now points to 0x1234A0. What happens to the Rectangle object at 0x123456? Is anything pointing to it anymore? Was it ever deleted?

All objects in memory created with the "new" command must have something that points to them or must be accessible in some way. If you change the only pointer to an object created with "new", you must explicitly "delete" that object prior to changing the pointer. And again, any pointer that DOES NOT point to anything should be NULL.

Look at line 90 again. obj points to a Rectangle. Then a new Circle is created and obj points to that. Where are you deleting the Rectangle that obj used to point to? Nowhere. That's why the Rectangle destructor is never called.

[hielo]

When obj.Set_draw ("Rectangle"); is executed, line 90 allocates memory for a Rectangle and obj points to it:

obj ---> +---+ Rectangle
         |   |
         +---+

You then execute: obj.Set_draw ("Circle");, resulting in:

         +---+ Rectangle
         |   |
         +---+

obj ---> +---+ Circle
         |   |
         +---+

Notice that what your line 90 did the second time around is to just allocate memory for the specified shape (Circle) and had obj point to it. The previous shape (Rectangle) is not destroyed because there is no automatic garbage collection in C/C++. You (the programmer) are responsible for keeping track of it and reclaiming it when no longer needed. Hence the reason you are not seeing the destructor being called -- it is still "hanging around" but you have no way to dereference (aks: memory leak).

You can fix this by initializing obj to nullptr. Whenever Set_draw() is called, delete whatever obj points to only if it is not nullptr - read comments in code below

class Draw
{
    private:
        IShape *obj;

    public:
        // provide a constructor that initializes obj to nullptr.  That way when
        // Set_draw() gets called the first time, you will know that if obj==nullptr
        // then there is no shape in obj yet, and hence there is nothing to delete.
        // After the first call, then you obj would have a reference to some shape
        Draw():obj(nullptr){}

        void Set_draw(string param)
        {
            // The first time you create a shape, this clause will be skipped
            // since initially obj==nullptr.  After the first object, this
            // clause will be executed.
            if( nullptr != obj )
            {
                delete obj;
            }

            // now assign a shape instance to obj
            obj=(IShape::create_instance(param ));
        }

        void Draw_Shape()
        {
            obj->draw();
        }

        ~Draw( )
        {
            cout<<"draw destructor"<<endl;
            delete obj;
        }
};

[can-mohan]

Thanks for your explanation. I had to reset the base pointer (by performing delete)similar to unique_ptr reset functionality before allocating other child class pointer .
upvote for this explanation.

[can-mohan]

One more thing which i would like to highlight here is NULL check against object
incase wrong type is passed to Set_draw fuction in order to avoid segmentation fault.

if (obj!=NULL)
      {
    obj->draw();
      }