Thread: Deleting dynamically allocated objects

  1. #1
    Daniel Jurnove
    Guest

    Deleting dynamically allocated objects

    I'm having a problem deleting objects I've created with the new operator. I have two classes: Particle and ParticleExplosion. ParticleExplosion has:
    vector< Particle * > pL

    Inside the constructor to the ParticleExplosion class, I make a whole bunch of Particles and put pointers to them in a vector ( pL ):

    ParticleExplosion::ParticleExplosion()
    {
    for( i = 0; i < this->numParticles; ++i )
    {
    p = new Particle;
    pL.push_back( p ); // pL is a vector
    }
    }

    When a ParticleExplosion object gets destroyed (by going out of scope), it's not taking the objects the pL points to with it (which I expected). So, I made my own destructor for ParticleExplosion, where I step through the vector, and delete each item that it poitns to:


    ParticleExplosion::~ParticleExplosion()
    {
    for( int i = pL.size(); i > 0; --i)
    {
    delete this->pL[i]
    }
    }

    After deleting the first item, the program crashes. Am I going about this the right way?

    [email protected]

  2. #2
    I lurk
    Join Date
    Aug 2002
    Posts
    1,361
    The vector<T>::size() returns the number of elements, but counting is zero based... it's the same with an array.

    vector<int> myvec;
    myvec.push_back(3);

    The size of the vector now equals 1, and myvec[0] is equal to 3.
    So, this might work a little better:

    Code:
    ParticleExplosion::~ParticleExplosion()
    {
        for( int i = 0; i < pL.size(); ++i)
        {
           delete this->pL[i]
        }
    }

  3. #3
    daniel jurnove
    Guest
    I had tried that, but it gave the same result: crashing as soon as I deleted the first object. I had thought that my problems might have been caused by deleting the first object in the vector, so I tried going from the end backwards. No luck there either.

  4. #4
    &TH of undefined behavior Fordy's Avatar
    Join Date
    Aug 2001
    Posts
    5,793
    Persoanlly I wouldnt do what you are doing.

    Containers of pointers are tricky as the destructors dont get called when the container goes out of scope.....there are other horrors as well so where possible try to avoid them (Scott Meyers says in his books to only try use pointers if you are using smart pointers...and those you will have to design yourself)....

    Another nasty is that calling push_back in a loop without reserving memory for the object is not usually the most efficeint way....and there's no need as you are using the default constructor for your Particle!!

    Add to this that if new throws an exception in say the 5th object created in the constructor, your destructor will not be called and you will leak the 4 objects previously created!

    Personally, I would initialise my vector to be created with a certain number of objects constructed from a default constructor for the Particle. You can do this in the constructor of ParticleExplosion, and the only overhead is that only 1 extra object will be created (the others are copied from this). The boon is that you will lose all the tedious deletion, it will be safer if an exception is thrown...and IMO its simpler

    Code:
    #include <iostream>
    #include <vector>
    
    class foo{
    public:
    	static int nObjCount;//Count of foo objects created
    	foo(){++nObjCount;}//Increase count for constructed obj
    	foo(const foo&){++nObjCount;}//Increase count for constructed obj
    	void Sing()const{std::cout << "Hello World" << std::endl;}
    };
    
    int foo::nObjCount;//Have to do this for static variable...sets to 0
    
    class bar{
    	std::vector<foo> MyVector;//Internal Array
    public:	
    	bar(int n):MyVector(n,foo()){}//Make n copies of foo() in array
    	const foo& operator[](int i){return MyVector[i];}//Address bar with []
    };
    
    int main(int nArg,char** szArg){
    
    	const int nNoOfObj = 10;//How many objects
    	
    	bar MyBar(nNoOfObj);//Make a bar with this many elements in array
    	
    	for(int i = 0;i < nNoOfObj;++i)
    		MyBar[i].Sing();//cal function for each element of array
    		
    	std::cout << "Total number of objects constructed ";
    	std::cout << foo::nObjCount << std::endl;//Count total number of object created for this	
    		
    		
    }
    Notice that for 10 objects needed, only 1 extra (11 in total) is created...well worth the cost of this 1 temp object to get a safer & simpler model IMO

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. dynamically allocated strings??
    By CS_Student8337 in forum C Programming
    Replies: 18
    Last Post: 03-19-2009, 05:14 AM
  2. Dynamically allocated size
    By maverickbu in forum C++ Programming
    Replies: 12
    Last Post: 06-26-2007, 01:16 PM
  3. Deleting Dynamically Allocated Array Members
    By theJ89 in forum C++ Programming
    Replies: 3
    Last Post: 03-26-2006, 07:37 PM
  4. deleting dynamically allocated memory...
    By heljy in forum C++ Programming
    Replies: 2
    Last Post: 09-08-2002, 11:40 AM