deleting element (objects) within a vector (not referenced by pointer) , memory handling

Question

This is an extension of this question . I have searched on the web but couldn't find a satisfactory answer.

I have a class A which defines a vector that contains a number of instances of class Region. The memory handling for these Region instances need to be managed by Class A

My questions are:

1. In the following code snippet, do I need to explicitly delete these instances in the desctuctor of class A?

2. Should I create instances as new Region(i) ? Is it better over the first option (not using new) Class A is quite a large object. How should I determine whether to use new or not?

3. If Answer to #2 is yes, how should I delete these instances in the destructor ? delete Region(i) (in a for loop) or delete [] reg?? I am guessing former way is the correct way, but want to confirm.

4. Other than "Region instances are not deleted" , do you see any obvious memory leak... especially in the construction of Region object in A's constructor?

class Region{
   public:
      Region(int num);
      int number;
      std::vector <elemt*> elements;   
      int info;
}

class A{
public:
    std::vector<Region> reg;
    const int numOfRegions = 100;
}

A::A(){
    int i;
    for ( i=0; i<numOfRegions; i++){
        reg.push_back(Region(i));
      } 
}

A::~A(){
  // How should the elements within vector Region be deleted??
  // Should I use "new" to allocate memory to instances of Region()
}

UPDATE: A BIG Thanks to all who answered!

Answer 1

1. The only thing you'll need to explicitly delete here in your example are the elemt* if the they are dynamically allocated. If so, they must be deleted by however owns them. In the case your Region only points to them, you should not delete them in its destructor.

2. First, Your vector is allocated in the stack and will be deallocated without problems. Second, to you use new Region(i) you would need to change your class A declaration. But there is not a necessity for that in your case. A few examples when you should use operator new:
1) the necessity for a more persistent data is required, since your dynamically allocated data will persist until explicitily deallocated (and acessible as long you keep a pointer refering to it)
2) your objects might be too big to be all stored in the stack.

3. If you did use new Region(i), you would to use operator delete at some point, probably on destructor or other cleanup function, to correctly deallocate this.

4. Your constructor would present a problem only if you are creating new elemt directly in your elements vector i.e. elements.push_back(new elemt()), for instance. This is a dangerous design that may cause several memory leaks or risks for seg faults. If you want to allocate them this way, you would need a smart pointer with support for copy mechanics like a shared pointer.

Answer 2

Inside class A everything seems to be defined OK. (You will need to tidy up the class declaration). This means that everything will be automatically destroyed when an instance of A is destroyed. Yes, and I mean that std::vector reg; destroys itself, and all the copies of Region instances it has responsibility for.

Are you really asking about class Region and its vector which has pointers, which, although you haven't shown any code for might need its pointers deleting?

edit:

( first edit included details of Region, second edit removes them and adds A::addRegions )

I think this is what you are after:

#include <vector>

class Region{
   public:
      Region(int num);
      // Details of Region removed
};

class A{
    std::vector<Region> reg;
    const int numOfRegions;
public:
    A();
    // No destructor needed: ~A();
    void addRegions(int num);
};

A::A():numOfRegions(100){
    for (int i=0; i<numOfRegions; ++i){
        reg.push_back(Region(i));
      } 
}
void A::addRegions(int num)
{
    for (int i=0; i<num; ++i){
         reg.push_back(Region(i));
    } 
}

Notice how class A has no destructor.

new

A::addRegions also uses push_back and even now no destructor is needed.

Also notice how class declarations have semi-colons after them, and the constructor for A initialises numOfRegions.

Answer 3

1. No. std::vector stores the Regions as objects and calls the necessary (de)constructors when elements are added/removed/overridden.

2. No. Only if you declare your vector as std::vector<Region *> reg;. You have to do that if Region is polymorphic or the copy/assignment constructors are expensive (vector copies elements around often).

3. If you store them as pointers you need to iterate over the reg and delete each element with delete ptr;.

4. Depending on how you create instances of the elemt-class you may want to add a destructor to Region to destroy these instances.

Answer 4

In the following code snippet, do I need to explicitly delete these instances in the desctuctor of class A?

No - they will be automatically deleted for you - if you don't create it with new, you don't delete it.

Should I create instances as new Region(i) ? Is it better over the first option (not using new) Class A is quite a large object. How should I determine whether to use new or not?

The size of A is not an issue. There is no clear answer to this question but the general rule is that if you don't have to create objects dynamically using new, then don't do so.

If Answer to #2 is yes, how should I delete these instances in the destructor ? delete Region(i) (in a for loop) or delete [] reg?? I am guessing former way is the correct way, but want to confirm.

If you created them using new, you would delete them in a for loop as:

 delete reg[i];

reg would have to be a vector of pointers of course. a better option would be to use a vector of smart pointers.

Other than "Region instances are not deleted" , do you see any obvious memory leak.. especially in the construction of Region object in A's constructor?

They are deleted - just not by you. This is known as RAII and is very, very good C++ practice - you should use RAII whenever possible.