CODEWITHSUNDEEP
c++
DarkUnderlord
DarkUnderlord

Reputation: 31

Deallocation of an array of objects?

I'm having some issues deallocating arrays of a class I have. Below is the Class, a simplified implementation and my code I have tried to use to close it.

Characters class

#include <cstdlib>


class Character
{
   private:

            bool human;
            int Xposition;  // the character's postion on the board.
            int Yposition;  // the character's postion on the board.
            bool alive;


   public:

            Character();        //This is my constructor
            ~Character();       //This is my destructor
            bool isHuman();     //return whether type 1 aka Human
            bool isZombie();    //return whether type 2 aka Zombie
            void setHuman();    //set to type 1 or Human
            void setZombie();   //set to type 2 or Zombie
            void setPos(int xpos, int ypos);        //set the board position
            int X(); 
            int Y();
            bool isAlive();     //checks to see if a Human is still alive and to be displayed
            bool Dead();        //kills the character and sets alive to false
            int num_moves_allowed; //number of moves allowed.


};

Allocation code:

Character *characters[11];
int human_count = 0;

for(int i=0; i<12; i++)
{
    characters[human_count] = new Character();
    human_count++;
}

Termination code:

for(i=11;i<=0;i--)
    {
        if(characters) 
        {
            characters[i]->~Character();
            delete characters[i]; characters[i] = NULL;
        }

    }
    if(characters) 
    {
            //ERROR IS HERE
            delete [] characters;
    }

I have tried a number of different "delete" commands on the array and I keep getting an "Debug Assertion Failed!" window. It says that the dbgdel.cpp from visual studio vctools is the problem place on Line 52.
It also says "Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)

Someone please help me I'm sure this is very simple.

Upvotes: 1

Views: 1916

Answers (5)

obelix
obelix

Reputation: 986

I'd suggest you avoid using arrays all together. Use a vector of characters.

Declare your vector as 

vector<Character> vCharacters;

then insert objects as 

for(int i = 0; i < 100; i++)
   vCharacters.push_back(Character());

If you want to store pointers to Character objects then wrap them in a shared_ptr which will take care of deallocating them for you.

vector<shared_ptr<Character>> vCharacters;
for(int i =0; i < 100; i++)
{
       shared_ptr<Character> spCharacter(new Character());
       vCharacters.push_back(spCharacter);
}

Avoid managing memory yourself when C++ can do it fo ryou

Upvotes: 10

Bill
Bill

Reputation: 14685

As obelix mentioned, you should use a vector from the Standard Template Library. However, if you're determined to use a raw array:

const int MAX_CHARACTERS = 11;
Character *characters[MAX_CHARACTERS];

for(int characterCount = 0; characterCount < MAX_CHARACTERS; ++characterCount)
{
  characters[characterCount] = new Character();
}

...

if (characters != NULL)
{
  for(int i = 0; i < MAX_CHARACTERS; ++i)
  {
    delete characters[i];
  }
}

Paolo Capriotti is correct that characters should be declared with new if you want it to last beyond its scope:

const int MAX_CHARACTERS = 11;
Character **characters = new Character*[MAX_CHARACTERS];

for(int characterCount = 0; characterCount < MAX_CHARACTERS; ++characterCount)
{
  characters[characterCount] = new Character();
}

...

if (characters != NULL)
{
  for(int i = 0; i < MAX_CHARACTERS; ++i)
  {
    delete characters[i];
  }
  delete [] characters;
}

A better solution is the standard vector class:

#include <vector>

...

const int MAX_CHARACTERS = 11;
std::vector<Character> characters;

for (int i = 0; i < MAX_CHARACTERS; ++i)
{
  characters.push_back(Character());
}

...

characters.clear();

Notice how much easier the cleanup was? (And in this case, it's optional, since when characters is destroyed it will automatically call the destructor of each item it contains.)

Upvotes: 2

ted_j
ted_j

Reputation: 319

i realize this is a simplified use and all, but why bother with heap access at all?

just using

Character characters[11];

could be just as valid, and safe.

std::vector<> is nice, but if the list is always fixed size, and there's no heap involved in member data, why not?

Upvotes: 0

pingw33n
pingw33n

Reputation: 12510

Also:

Character *characters[11];

should be

Character *characters[12];

and

for(i=11;i<=0;i--)

should be

for(i=11;i>=0;i--)

Upvotes: 0

Paolo Capriotti
Paolo Capriotti

Reputation: 4072

The characters array was allocated on the stack, so you don't have to delete it. However, if you want the array to survive the local scope, create it with something like this:

Character **characters = new Character[11];

then your delete[] line should work fine.

Also note that you don't need to call the destructor of Character explicitly: it is called automatically by delete.

Upvotes: 5

Related Questions