Sign in to follow this  
vinb

error releasing memory

Recommended Posts

How would I release this memory...
Deck *theDeck = new Deck() ;
...
delete [] theDeck ;

class Card{

private:
	char suit ;
	char name ;
	int value ;

public:
	
	Card();
	~Card();

	char getSuit() ;
	void setSuit(int i) ;
	char getName() ;
	void setName(int i) ;
	int getVal() ;
	void setVal(int i);
};

Card::Card(){
	this->value = 0 ;
	this->name = ' ' ;
	this->suit = ' ' ;

}


class Deck{

private:
	int index ;
	Card card[51] ;

public:
	Deck();
	~Deck();

	Card getCard(int card) ;
	void deal() ;
	int getIndex() ;
	
} ;

Deck::Deck(){
	
	const int DECK_SIZE = 52; 
	for (int i=0;i<=DECK_SIZE;i++){
		
        card[i].setVal(i) ; 
        card[i].setSuit(i) ;
        card[i].setName(i) ;
		
		cout << card[i].getName() << "  " << endl ;
		cout << card[i].getSuit() << "  " << endl ;
		cout << card[i].getVal() << "  " << endl ;
		
	} 

}

I keep getting a nasty unhandled exception error. I think it has something to do with the fact that the Deck object contains an array of objects that isn't getting released properly, but I can't seem to nail it down. Can anyone make sense of this?

Share this post


Link to post
Share on other sites
Don't use delete [] unless you allocated an object with new []. For single object allocation, simply use delete:



Deck *theDeck = new Deck();

...

delete theDeck;



The cards array in the Deck class doesn't have anything to do with this error.

Magius

Share this post


Link to post
Share on other sites
Tried that. I still get the error. Actually, I started by using delete without the []. I figured since the object contained an array, I needed to use the brackets. Do I need to do some cleanup in the destructor?

Share this post


Link to post
Share on other sites
You have a bound error in your Deck class/constructor:

- card[51] is an array that contains 51 objects of type Card.
- your for( i = 0; i <= 52; ++i ) loop will run from card 0 to card 52, which is 53 cards (and overflows the array).

May I suggest using a std::deque, and then using a for( i = 0; i < 52; ++i ) to push_back your objects into the deque?

Share this post


Link to post
Share on other sites
No, you don't need to do any cleanup in the destructor for Deck. There is a difference between heap memory and stack memory. You only need to cleanup heap memory, stack memory is automatically cleaned up when whatever object or variable loses scope. Heap memory is any memory you allocate with the "new" command. So, if you don't use the "new" command to allocate, then you don't need to use the delete command to deallocate. Likewise, if you allocate memory with "new x[]", then you have to deallocate it with "delete []". If you allocate with "new", you deallocate with "delete".

I'm not sure what is causing your unhandled exception. There are a couple of things you are doing that are questionable, and the code you posted obviously isn't the code you have since there would be numerous undefined types and variables. Do you use your "theDeck" variable anywhere before you use the "new" command to create it?

[edit] oh yeah, there was a bounds error... didn't even see that [/edit]

Share this post


Link to post
Share on other sites
Using the std library was suggested to me, but I wanted to get practice using classes. I didn't even notice the <=52 . Thanks. Unfortunately, it still crashes.

Share this post


Link to post
Share on other sites
Quote:
Original post by Tibre
Do you use your "theDeck" variable anywhere before you use the "new" command to create it?


Nope. I'm doing this incrementally. I just create the objects and then delete them. In the constructor of the Deck I am cout-ing the name, value and suit just to make sure they are getting values. Here's that code..

Deck::Deck(){

const int DECK_SIZE = 52;
for (int i=0;i<DECK_SIZE;i++){

card[i].setVal(i) ;
card[i].setSuit(i) ;
card[i].setName(i) ;

cout << card[i].getName() << " " ;
cout << card[i].getSuit() << " " ;
cout << card[i].getVal() << " " << endl ;

}

}



Also, am I understanding you to mean that memory allocated within a class is created on the stack? I assumed that because this is a pointer to a Deck object, that its members would also be created on the heap. In the constructor, should I be doing something like this...

for (i=0;i<51;i++) this->card[i] = new Card() ; 

Share this post


Link to post
Share on other sites

...
class Deck{

private:
int index ;
Card card[51] ;
...




Even if you change the for-loop to use < DECK_SIZE instead of <= you're still overflowing the buffer. Your deck only has 51 cards and you're trying to set values for a 52:nd card. Either change DECK_SIZE to 51 or or change the deck to contain 52 cards.

If you define DECK_SIZE outside the constructor and before the class definition you could use it for both the array size (Card card[DECK_SIZE];) and in the for-loop. That way you'd only have to change the size in one place.

Or better yet you could use a standard library container such as std::vector.

std::vector<Card> cards(DECK_SIZE);

would do the trick.

Share this post


Link to post
Share on other sites
Quote:
Original post by vinb
Also, am I understanding you to mean that memory allocated within a class is created on the stack? I assumed that because this is a pointer to a Deck object, that its members would also be created on the heap. In the constructor, should I be doing something like this...


I'm sorry, guess I'm not all together this morning. Your assumption about the member variables is correct. You don't need to call new for the card's inside of your deck, unless you want them to be dynamically allocated separately from the deck.

I misspoke earlier. Let me rewrite that more correctly.

When you call 'new Deck()', you will allocate all of the memory for a Deck class object. This includes allocating enough memory for 51 (should be 52 as pointed out by fiskbil) cards. So you don't need to call new or delete on them. Likewise, when you call 'delete theDeck' you are deallocating all of the memory you allocated with the 'new Deck()' call, including the memory for the array of cards.

You're right that the memory for those cards is allocated on the heap, however it's all allocated in chunk that belongs to the 'Deck class', and so you only have to delete the deck. You would only have to delete that memory with a separate delete call, if it was allocated with a seaparate new call.

Share this post


Link to post
Share on other sites
You guys are absolutly awsome! As it turns out, it was not allocating enough size for the Card array within the Deck that was the problem. If I understand what was going on, I allocated enough space for 51 cards and then proceded to fill 52 slots. When I went to delete the pointer, there was still some unaccounted for memory hanging out there and crash and burn. Thanks so much for helping me work this out.

Share this post


Link to post
Share on other sites
Actually, I'm pretty sure the crash happened at the very moment where you wrote to the 52nd card.

You can check this by cout'ing something just before you try to delete the object, and see if it is displayed.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this