CODEWITHSUNDEEP
c++memory-leakspolymorphismc++98
rle
rle

Reputation: 21

How to avoid memory leaks with polymorphism and vector?

Good morning,

I am working on a C++ application which uses polymorphism and I would like some advice in order to avoid memory leaks. See below the code :

class IItem
{
public:
  IItem(){}
  virtual void Init() = 0;
  virtual ~IItem(){}
};

class ItemA : public IItem
{
private:
  int m_a;

public:
  ItemA(){ m_a = 0; }
  void Init(){m_a = 10;}
  virtual ~ItemA(){}
};


class Element
{
public:
  int m_a;
  int m_b;
  IItem* m_pItem;

  Element(){}
  void Init(IItem* item)
  {
    m_pItem = item;
  }
  virtual ~Element(){}
};

class ItemInfo
{
public:
  int m_id;
  std::string m_name;
  Element m_element;

public:
  ItemInfo(){}
  virtual ~ItemInfo(){}
};

class Test
{

public:
  Test(){}
  virtual ~Test(){}

void Initialize(std::vector<ItemInfo>* arrayItem)
{
    for(int i=0;i<5;i++)
    {
            arrayItem->push_back(ItemInfo());
            arrayItem->back().m_element.Init(new ItemA());
    }
 }
};

And I am calling in a main program with this line :

Test test;
std::vector<ItemInfo> arrayItemInfo;
test.Initialize(&arrayItemInfo);

In the function Initialize I am using "new ItemA" and my question is how to delete property the memory ?

Thank you in advance for your help.

Upvotes: 1

Views: 223

Answers (1)

Mateusz Grzejek
Mateusz Grzejek

Reputation: 12098

If you can, use std::unique_ptr<> instead of plain pointer (*):

class Element
{
public:
  std::unique_ptr<IItem> m_pItem;

  Element(){}
  void Init(IItem* item)
  {
    m_pItem.reset(item);
  }
  virtual ~Element(){}
};

Note, that in this form, your class becomes non-copyable (because std::unique_ptr has deleted copy semantics). If this is a limitation, you can:

(*) Of course you need to create objects using new (as you do), because default deleter for std::unique_ptr simply deletes the pointer. If you want different behavior, you can provide custom deleter.

If you cannot use C++ 11 features, the easiest thing would be to manually handle the memory management in destructor:

class Element
{
public:
  IItem* m_pItem;

  Element(IItem* item)
  : m_pItem(item)
  { }

  virtual ~Element()
  {
    delete m_pItem;
    m_pItem = NULL;
  }
};

class ItemInfo
{
public:
  int m_id;
  std::string m_name;
  Element m_element;

public:
  ItemInfo(IItem* item)
  : m_element(item)
  { }
  virtual ~ItemInfo()
  {}
};

I also updated the code to use initialization via constructors instead of ugly Init/Release - like interface. Now, with this code, your loop becomes way better:

void Initialize(std::vector<ItemInfo>* arrayItem)
{
    for(int i = 0; i < 5; i++)
    {
      arrayItem->push_back(ItemInfo(new ItemA()));
    }
}

Note, however, that you need to provide proper copy semantics to avoid multiple deletion of the same object (or simply disable copying).

Another way may be to look at the std::auto_ptr<>, but since it's now deprecated (as of C++ 11) I would suggest to avoid using it.

By the way - virtual destructors in Element, ItemInfo and Test classes are not necessary in the code you posted.

Upvotes: 2

Related Questions