views:

100

answers:

8

Take a simple class with the "big 3" (constructor, copy constructor, destructor):

#include <vector>
using namespace std; //actually goes in the C file that links to this header file
...
class planets(){ //stores mass and radii data for planets in a solar system.
   public:
      vector <double> mass;
      vector <double> radius;

   //constructor
   planets( int numObj ){
     for(int i=0; i<numObj; i++){
         mass.push_back(8.0); //some default values.
         radius.push_back(2.0);
     }
   }
   //copy constructor
   planets(const planets &p){
      vector <double> mass(p.mass); //copy vectors into new class.
      vector <double> radius(p.radius);
   }
  //destructor
  ~planets(){
     delete mass; //ERROR: (...) argument given to ‘delete’, expected pointer
     ~radius(); //also causes error: no match for call to(...) 
   }
}

I plan on making a vector of planets, thus the need for the "big 3":

vector <planets> stars;
stars.push_back(planets(5)); //5 hypothetical planets of alpha centauri
stars.push_back(planets(8)); //our solar system. Used to be nine.
///etc.

How do I delete the mass and radius vectors properly, to avoid memory leaks (do I even have to)?

+3  A: 

You don't have to implement any destructor for your class. The vectors will be automatically destroyed. This is related to the Resource Acquisition Is Initialization pattern.

kotlinski
+1  A: 

No - you don't have to. member variable's destructors are automatically invoked after the containing object's destructor.

Tony
+5  A: 

No, you don't need to do anything because you aren't managing any resources. You only write the Big Three when you're managing a resource, but vector is doing that. It's the one with the Big Three properly written, you just use it.

This is why the single responsibility principle is key in resource management: once you have some class that properly manages a resource, you can simply use it without ever worrying about that resource again. Always split resource management from resource use.

The reason you need the Big Three written in a managing class is because the default special members typically do the wrong thing (they copy, assign, destruct values instead of what the values manage/point at.) But once you're resource is wrapped up (like in a std::vector), everything is just fine. The defaults will copy vector, but that copying is correctly written.

By the way, the Big Three is in the context of managing resources (copying and destroying resources), not created them. So it would be copy-constructor, copy-assignment, and destructor, not default constructor.


For your information, here's how you would do it:

class planets
{
public:
    // ...

    //copy constructor
    planets(const planets &p) : // use an initialization list to initialize
    mass(p.mass), // copy-construct mass with p.mass
    radius(p.radius) // copy-construct radius with p.radius
    {
        // what you had before just made a local variable, copy-constructed
        // it with p.xxx, then got released (nothing happened to your members)
    }

    //destructor
    ~planets()
    {
        // nothing to do, really, since vector destructs everything 
        // right for you, but you yes, you would delete any resources
        // you managed here
    }
};

But don't forget the copy-assignment operator. I recommend the copy-and-swap idiom, and leave that as an exercise for you.

(Remember you don't actually need these, though.)

GMan
A: 
  ~planets(){ 
     mass.clear();
     radius.clear();
  } 

the above should be enough as your member vector objects do not have any pointers.

Chubsdad
even that is not required. `vector` is anyway going to release the memory in its destructor.
Naveen
I know that. I was trying to show the OPoster the crrrect way of doing things in the destructor. delete mass and ~radius are not correct.
Chubsdad
Why down votes?
Chubsdad
@chubsdad: I imagine people are downvoting because `clear` is _not_ required (as @Naveen said) since the vector destructors will get called. Putting in a call to `clear` shows a lack of understanding of that basic point.
Troubadour
A: 

In your particular situation you don't have to! You're not storing pointers in the vector. You're not storing pointers to vectors in your planets class(that is you've not dynamically allocated the vector<> object so why trying to delete it).

celavek
A: 

As you didn't newed mass, you don't need to delete it.

Also, the destructors of mass and radius will be called automatically, you don't need to call them explicitely

Didier Trosset
+3  A: 

The 'big three' aren't what you say they are. They are: copy constructor, copy assignment operator and destructor.

vector instances are already copiable and assignable, you don't have to do anything special so with two members of type vector<double> you don't need to provide custom implementations of the big three.

Your copy constructor is incorrect, it doesn't copy the source vectors in to the new class, it just constructs function locals from them which are then discarded. This creates local variables called mass and radius which mask the member variables with the same name.

planets(const planets &p){
  vector <double> mass(p.mass); //copy vectors into new class.
  vector <double> radius(p.radius);
}

More correct (but unnecessary) would be:

planets(const planets &p)
  : mass(p.mass) //copy vectors into new class.
  , radius(p.radius)
{
}

Similarly your destructor body should be empty. You only delete pointers which have been allocated with new. As you have straight member variables no special action is required.

Charles Bailey
But even more correct would be not use the compiler generated default version.
Martin York
A: 

Your class can be simplified too:

class planets()
{ //stores mass and radii data for planets in a solar system.
  public:
    std::vector<double> mass;
    std::vector<double> radius;

  //constructor
  planets( int numObj )
  {
    for(int i=0; i<numObj; i++)
    {
      mass.push_back(8.0); //some default values.
      radius.push_back(2.0);
    }
  }
}

Your class does not contain any resources (ie pointers)
Therefore you do not need to explicitly manage them. Each class is supposed to know how to:

  • Copy Itself
  • Be assigned over
  • Destroy itselft

The compiler generated copy constructor assignment operator and destructor will automatically call these operations on any member variables (mass and radius) so you don't need too. So in your case the std::vector knows how to correctly do all three operations and therefore you do not need to add any extra code.

Martin York