0

I have a class A like:

class A
{
   int a;
}

And, Also I have class B that inherited class A:

class B : public A
{
   int b;

public:
   static A** ReturnAPtrArray(int size);
}

Then, I make Array having A class Pointer in class B.

A** B::ReturnAPtrArray(int size)
{
   A** array = new A*[size];
   for(int i = 0; i< size; i++)
   {
      array[i] = new A();
   }

   return array;
}

In main func, I Called class B's ReturnAPtrArray() func.

void main(void)
{
   int size = 100;
   A** aptrArray = B::ReturnAPtrArray(size);

   --------Do Something

   delete[] aptrArray;
}

This main func makes memory leak. So I deleted every pointers like this:

void main(void)
{
    int size = 100;

    A** aptrArray = B::ReturnAPtrArray(size);

    --------Do Something

    for(int i = 0; i< size; i++)
    {
       delete aptrArray[i];
    }

    delete[] aptrArray;
}

After modified main func, memory leaks were disappeared.

If I want to free memory, should I delete all pointers in Pointer Array?

Is there any other options?

Samuel Liew
  • 76,741
  • 107
  • 159
  • 260
Retalia K
  • 64
  • 9
  • add a virtual destructor to class A, see https://stackoverflow.com/questions/461203/when-to-use-virtual-destructors – Marc Stroebel Jan 07 '20 at 10:34
  • 2
    One `delete` for each `new` and one `delete[]` for each `new[]`. – Yksisarvinen Jan 07 '20 at 10:34
  • How existance of class `B` related to the question at all? – Slava Jan 07 '20 at 10:37
  • 2
    `std::vector>` to avoid manual memory management. – Jarod42 Jan 07 '20 at 10:39
  • 1
    Yes. Every usage of a `new` expression must be matched by a corresponding `delete` expression. The allocation function uses `new[]` exactly `size + 1` times. To release, it is necessary to use the `delete []` on all of those `size + 1` allocations (i.e. every pointer given by a `new []` expression must be explicitly fed to a `delete []` expression exactly once). – Peter Jan 07 '20 at 10:40
  • And, BTW, this has nothing to do with inheritance, since your `B::ReturnAPtrArray()` only creates objects of type `A` in the loop. By describing the problem as you have, you are obfuscating the REAL problem - the fact that you have derived `B` from `A` is irrelevant to your problem as described. If `B` was not derived from `A` the required behaviour in `main()` would be identical. – Peter Jan 07 '20 at 10:43
  • @Slava I have many class C,D,... that inherited class A. And its are excuted other ways. I lacked explain, it's my mistake – Retalia K Jan 07 '20 at 10:44
  • 1
    So you use pointers because you may have pointers to a derived class? Then `class A` must have virtual destructor, otherwise `delete aptrArray[i];` may have Undefined Behaviour. – Slava Jan 07 '20 at 10:46

2 Answers2

3

If I want to free memory, should I delete all pointers in Pointer Array?

Yes you should

delete[] deletes only the array it self. Since you have a array of pointers you must delete every pointer element individually.

As for other options you can use smart pointers.

Example:

#include <memory>
#include <vector>

int main()
{
    std::vector<std::shared_ptr<A>> array;
    for(int i = 0; i < 100; i++)
    {
        array.push_back(std::make_shared<B>());
    }
}

when the array goes out of scope it deletes it self

Nathan Kolpa
  • 89
  • 1
  • 11
  • 1
    It is a good answer, however, is there really a justification for using shared_ptr, see http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f27-use-a-shared_ptrt-to-share-ownership. – Tiger4Hire Jan 07 '20 at 11:20
0

It is important to distinguish between polymorphic-ownership and polymorphic use. Polymorphic ownership is when you want to own a thing (or many things) that are are of an unknown type. Polymorphic use is when you want to manipulate a thing when you don't know what it is. Your example doesn't really make clear why you use inheritance at all, so I will explain both.

If you only create B's just declare them as B's. If you want to pass a set of B's to a function that doesn't know they are B's, the simply create a vector of B's and pass them as pointer-to-A.
Like this ...

std::vector<B> myBs(5);   // create 5 default B's
for (const auto& thisB: myBs) Fn(&thisB); // cast a ptr-to-B to a ptr-to-A

Keeping it simple like this will make your life a lot simpler, as this code is type-safe.

If on the other hand you want to own a list of things that might be a B, or might not, but definitely inherits from A. Use something like this.

std::vector<std::unique_ptr<A>> my_maybe_bs_might_not;

This pattern might seem superficially simpler, but it comes with a lot of gotcha's. For example, you must use must use a virtual destuctor. This in turn invokes the rule-of-3/5. Simply put, if the compiler doesn't know a thing is, you have to tell it how it can be moved/copied. If you don't, the compiler will probably do the wrong thing.

A simpler (assuming you have C++17) scheme for polymorphic-ownership is using a variant. When you use a variant, you must list all things it might be, this can include smart pointers.

using MaybeB = std::variant<std::unique_ptr<B>, std::unique_ptr<C>>;
std::vector<MaybeB> my_maybe_bs_might_not;

This pattern allows the compiler to generate all the right code for you, while making it simple to add new classes. The only downside is that because you must list all the things you might want to own. This makes it a bad choice for a library-level system, where the user of the library might want to write their own class (derived from A) and add it to your list.

The general advice is to pick the simplest scheme possible, which in general means avoiding polymorphic ownership unless that is really required.

Tiger4Hire
  • 1,065
  • 5
  • 11