1

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.

Improve this question
3
  • 5
    You should never call the destructor explicitly. That's done automatically when you delete the object. Commented Aug 28, 2009 at 19:28
  • 1
    On a side note, are those your actual comments or post-ified? Commented Aug 28, 2009 at 20:22
  • comments in the code above I put in, some just for your guys. This is still in very early dev. I'm the only one working on it so if some of it is wrong from cutting and pasting, I could care less right now. ;) Commented Aug 29, 2009 at 3:39

5 Answers 5

Reset to default
10

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

Improve this answer
Sign up to request clarification or add additional context in comments.

10 Comments

honestly I've never used vectors before in C++. I first learned C++ back around 1995. I need to be able to get to these arrays quickly to check properties etc for the arrays.
So are you assuming vectors are slow or don't let you do that?
I agree with your answer (and upvoted it), but please - no Hungarian notation.
"This of course will not create you Character objects on the heap" - yes it will. The contents of vectors are always created on the heap.
@Neil: my bad. have edited the post. what I meant was that @DarkUnderlord had explicitly tried to new up his objects on the heap and if that is the behaviour he wants he should use shared_ptr to wrap the pointers instead of allocating and deallocating them manually
|
5

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.

Improve this answer

3 Comments

LOL I know about the destructor automatically running, I was just at my end of trying anything. :) Now I'm getting "Error 5 error C2440: '=' : cannot convert from 'Character *' to 'Character **' on the line that says "characters[human_count] = new Character();"
I'm tracking players on a grid (like chess) so it's important I can loop through them quickly and align the loop variable to another array at the same time.
std::vector, when compiling with optimizations enabled, is exactly as fast as a plain dynamic array for all indexing operations.
2

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

Improve this answer

3 Comments

The second example is wrong. "new Character[MAX_CHARACTERS]" creates an array of Character objects, but then you go through and manually create an array of Character objects, overwriting (and leaking) the first ones. "delete [] characters" will try probably crash since it's deleting an array of already-deleted objects.
Never mind my comment - there was a typo in the example. It should have been "new Character *[MAX_CHARACTERS]", which is allocating an array of pointers, not an array of objects. I fixed the example and removed my downvote. :-)
Ah, thanks! I was writing in a hurry, and didn't proofread as well as I should have.
0

Also:

Character *characters[11];

should be

Character *characters[12];

and

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

should be

for(i=11;i>=0;i--)
Improve this answer

Comments

0

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?

Improve this answer

2 Comments

In this case it's probably fine, but be careful if you have large classes.
It also requires that Character be default constructible, which vector does not. In this case it is - wrongly, I think.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.