How to avoid memory leaks with polymorphism and vector?

Question

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.

You have many options. 1) use smart pointers. 2) use the RAII design pattern. Besides that, the question is too broad. — Sam Varshavchik, Feb 13 '18 at 13:31
(1) Write constructors, not `Init` functions. What you do is an anti-pattern in C++. (2) Your classes have destructors. It's a perfect place to do resource release. It's such a popular C++ idiom, [we coined a named for it](http://en.cppreference.com/w/cpp/language/raii). — StoryTeller - Unslander Monica, Feb 13 '18 at 13:31
https://stackoverflow.com/questions/106508/what-is-a-smart-pointer-and-when-should-i-use-one — John Zwinck, Feb 13 '18 at 13:33
https://stackoverflow.com/questions/4782757/rule-of-three-becomes-rule-of-five-with-c11 — UKMonkey, Feb 13 '18 at 13:40
Concerning using smart pointers, I am working on an old application then I have to use "basic" c++. I will add resource release in the destructors but the problem is that I make a "new" inside another element. — Zaraki21, Feb 13 '18 at 14:13

Mateusz Grzejek · Answer 1 · 2018-02-13T14:31:15.457

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:

provide custom copy/move semantics for your class (rule of three/five) to implement copy/move as you want/need.
used std::shared_ptr<> instead if data can be shared.

(*) 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.

Thank you for your answer but the problem is that I can't use C++ 11 in my application. — Zaraki21, Feb 13 '18 at 14:17

How to avoid memory leaks with polymorphism and vector?

1 Answers1