Deallocation of an array of objects?

Question

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.

Answer 1

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.

Answer 2

Also:

Character *characters[11];

should be

Character *characters[12];

and

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

should be

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

Answer 3

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

Answer 4

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.)

Answer 5

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?