Link to home
Start Free TrialLog in
Avatar of __fletcher__
__fletcher__

asked on

Deallocation of memory

I have a subclass called CardDeck, which extends a base class CardPile.  In my default constructor in CardDeck, I create 52 const Card*'s, and call the addCard(crd) method from CardPile so that they are added to the state in CardPile.  My CardPile has an empty destructor, since it never dynamically allocates memory, however, my CardDeck does(those 52 Card*'s).  How should I go about coding my destructor to get rid of those cards from a state that's not even in CardDeck, it's in CardPile.  
P.S.  the state is a vector<const Card*>
Avatar of alexo
alexo
Flag of Antarctica image

class CardPile
{
public:
    // ...
    virtual ~CardPile() {}
};

class CardDeck: public CardPile
{
public:
    // ...
    virtual ~CardDeck();
};
When the root class has a virtual destructor (even empty), the correct destructor will be called.  E.g.:

CardPile* p = new CardDeck;
// ...
delete p; // CardDeck::~CardDeck() will be called.
Avatar of __fletcher__
__fletcher__

ASKER

That makes sense, but my destructor in CardDeck shouldn't be empty, and I don't know what to put in it.  The CardDeck constructor allocates memory for 52 Card *'s, so how do I code my destructor in CardDeck to deallocate that memory?
...at least I don't think it should be empty.(I don't really know)
That depends on how and what you allocate.
Post the relevant source.
public CardDeck: public CardPile {
  typedef vector<const Card*> card_list;
  typedef size_t size_type;
  static const size_type npos = static_cast<size_type>(-1)

public:

CardDeck() {

const Card* acespades = new Card(Spades, Ace);
//... 52 times ...//

addCard(acespades);
//...52 times ...//

myCards() = getCards();  //getCards returns a card_list&
}

//...//

virtual ~CardDeck() {
//do I need something here?
}

//...//

private:

card_list& myCards() { return _deck_of_cards; }
size_type& mySize() { return _size; }

card_list _deck_of_cards;
size_type _size;
};

CardPile has a similar structure, but it creates an empty pile as a default constructor, that's why I create 52 cards and add them.

So...the cards are added to the pile state, as well as the _deck_of_cards state, which are both vectors.  How(or do i need to)do i deallocate that memory??

Thanks!!

P.S. the addCard method is not virtual, and belongs only to the CardPile class.  I may change that later, but for now, that works.
   const Card* acespades = new Card(Spades, Ace);
    //... 52 times ...//

    addCard(acespades);
    //...52 times ...//

<boggle>

Welcome to the wonderful word of software engineering.  For your safety, please observe the following rules:
- Don't hardcode.
- Don't duplicate.
- Don't try to scratch behing your left ear with your right elbow.

Assuming you need a deck of 52 cards, try something like:

    enum Suite { Spades, Diamonds, Clubs, Hearts };
    enum Value { Ace = 1, Two, Three, Four, Five, Six, Seven, Eight, Nine, Ten, Jack, Queen, King, Ace };

    for (Suite suit = Spades; suites <= Hearts; ++suit)
        for (Value value = Ace; value <= King; ++value)
            addCard(Card(suit, value));

No additional variables, no allocations...
>> the addCard method is not virtual, and belongs only to the CardPile class.

Sounds right.
but addCard takes a pointer as an argument.

will this still work?

for(...)
  for(...)
   addCard(&Card(suit,value));

??

thanks for your help alexo
i got the following errors when I attempted your suggestion:

>>no match for '++Suit &'
>>no match for '++Value &'
>>warning: taking address of temporary

I assume the last one means that I need to allocate memory to make those cards stick around?
>> addCard(&Card(suit,value));
Won't work (at least it is not standard C++).

Why not pass a reference instead of a pointer?  That is the preferred way in C++.  Unless it is an assignment and you haven't learned references yet.

Hmmm...

    Card* pCard; // Please, no TNG jokes.
    for (Suite suit = Spades; suites <= Hearts; ++suit)
    {
        for (Value value = Ace; value <= King; ++value)
        {
            pCard = new Card(suit, value);
            addCard(pCard);
            delete pCard;
        }
    }

Or

    Card card;
    for (Suite suit = Spades; suites <= Hearts; ++suit)
    {
        for (Value value = Ace; value <= King; ++value)
        {
            card = Card(suit, value);
            addCard(&card);
        }
    }

Or

    Card card;
    for (Suite suit = Spades; suites <= Hearts; ++suit)
    {
        for (Value value = Ace; value <= King; ++value)
        {
            card.setSuit(suit);
            card.setValue(value);
            addCard(&card);
        }
    }

All of the solutions are convoluted (the 1st one is the worst).  Passing by reference is really the best option (a fourth solution will be passing the card by value, also not optimal - a copy constructor is invoked -  but looks cleaner).
>> i got the following errors when I attempted your suggestion [...]

Damn, I must be tired today...
You cannot increment an enum.  Try:

   for (int suit = Spades; suites <= Hearts; ++suit)
       for (int value = Ace; value <= King; ++value)
           addCard(Card(suit, value));

Also, the rest of the examples :-(
These comments are good suggestions, but they don't fix my problem.  I need a way to set the state in CardPile to have 52 Cards*'s, which I do by allocating space for each one(I need to do this, because they need to last longer than the scope of the constructor), and also I need to set the state in CardDeck to have 52 Card*'s(done the same way).  Since CardDeck IS a CardPile, the reason I do this is to keep track of a specific deck in a pile that can have more than 1 deck. (i.e. the pointers reside in 2 places, a deck and a pile, so that a duplicate card in a pile won't be mistaken for a card that belongs to a specific deck).

I can get all this to work, by creating 52 card*'s, and adding them using the CardPile::addCard(crd) method, but if I do that, then I'm going to have to de-allocate them, and I don't know how to do that.

Thanks again for the help!

P.S. What is a TNG joke?(I hope I didn't make one and insult you or something! I really do appreciate the help!)
also, my Card(Suit_Type, Face_Value); constructor takes enum's as arguments, so I can't pass in int's.(So that wont work...thanks anyways!)
A simple rule would be, the one who allocates them has to delete them.

A less simpler rule is: Don't allocate at all.
Containers are here to handle such problems, not to make them worse. Try typedef vector<Card> card_list . As long as you don't add new cards to the vector or remove elements all pointers to the vector remain valid.

Basicly, Avoid using pointers to share references, use pointers if you need to change references at runtime.
If you want to share references, you can use boosts shared_ptr instead.
ASKER CERTIFIED SOLUTION
Avatar of alexo
alexo
Flag of Antarctica image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
>> also, my Card(Suit_Type, Face_Value); constructor takes enum's as arguments.

You can always cast...
just on a sidemark, do make clean stl code if you gonna do a loop over the pointer array.

typedef card_list::iterator clIter;

for( clIter i=myCards.begin(); i!=myCards.end(); i++ )
{
  delete i;
}
sry, forgot something

ust on a sidemark, do make clean stl code if you gonna do a loop over the pointer array.

typedef card_list::iterator clIter;

for( clIter i=myCards.begin(); i!=myCards.end(); i++ )
{
 delete (*i);
}
>> also, my Card(Suit_Type, Face_Value); constructor takes enum's as arguments.

You can always cast...
In short, having 52 distinct variables is a bad thing(TM), an array (or container) is much cleaner and is worth some forcing of enums.

Just an idea:
    Suit nextSuit(Suit suit) { return Suit(int(suit) + 1); }

If you want a really clean solution - make Suite and FaceValue classes (with bounds checking, ++/-- operations, etc.)
THANKS FOR ALL YOUR HELP!!!!!
allocating and deallocation memory problem. Hmm.. the Achilles' heel of C++ programmers.. Nobody gets it right all the time. If you treat memory as a resource, there is a simple rule that will work most of the time.

1. Allocate resources in constructors
2. Deallocate/Release resources in destructors

With this approach, you can safely rely on C++ to do the job. Because C++ guarantees that destructors to all COMPLETLY constructed objects will be called once they leave their scope. Even if an exception occurs.
 
If you use smartpointers such as auto_ptr in STL (without referencecounter) or boost::shared_ptr (www.boost.org) (with ref. counter) to contain your memory allocation, you never have to write 'delete' explicitly in your code.

Don't you think it is nice?

See more about resources in www.relisoft.com 

"Cover all 'naked' new with a smartpointer"

example:

********************************

// naked version
{  // scope - start

 C *p = new C;
 ...
 ...
 delete p; // must write this

} // scope - end

********************************

// covered version

{ // scope - start

   auto_ptr<C> ap_C = auto_ptr<C>(new C);
...
...

} // scope - end, no delete written

*********************************