Deallocation of memory

__fletcher__
__fletcher__ used Ask the Experts™
on
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*>
Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®

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

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

Commented:
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.

Author

Commented:
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?
Bootstrap 4: Exploring New Features

Learn how to use and navigate the new features included in Bootstrap 4, the most popular HTML, CSS, and JavaScript framework for developing responsive, mobile-first websites.

Author

Commented:
...at least I don't think it should be empty.(I don't really know)

Commented:
That depends on how and what you allocate.
Post the relevant source.

Author

Commented:
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!!

Author

Commented:
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.

Commented:
   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...

Commented:
>> the addCard method is not virtual, and belongs only to the CardPile class.

Sounds right.

Author

Commented:
but addCard takes a pointer as an argument.

will this still work?

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

??

thanks for your help alexo

Author

Commented:
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?

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

Commented:
>> 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));

Commented:
Also, the rest of the examples :-(

Author

Commented:
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!)

Author

Commented:
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!)
EOL

Commented:
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.
Commented:
If a deck isa pile then why do you need double pointers?
Consider: a pile has a list of N (arbitrary?) cards.  A deck has the same list, only that the cards have more constraints on them (N==52 and the cards are unique).

Now to the other issue.  In general, whever allocates a resource should be the one to free it.  If you want the cards to be on the heap and the addCard() func will just manage the pointers, you can do something like:

class CardDeck
{
public:
    CardDeck();
    ~CardDeck();
private:
    enum { NumCards = 52 };
    Card* myCards[NumCards]; // Full deck
};

CardDeck::CardDeck()
{
    int counter = 0;
    for (int suit = Spades; suites <= Hearts; ++suit)
    {
        for (int value = Ace; value <= King; ++value)
        {
            myCards[counter] = new Card(suit, value);
            addCard(myCards[counter]);
            ++counter;
        }
    }
}

CardDeck::~CardDeck()
{
    for (int counter = 0; counter < NumCards; ++counter)
        delete myCards[counter];
}

Commented:
>> also, my Card(Suit_Type, Face_Value); constructor takes enum's as arguments.

You can always cast...
EOL

Commented:
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;
}
EOL

Commented:
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);
}

Commented:
>> also, my Card(Suit_Type, Face_Value); constructor takes enum's as arguments.

You can always cast...

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

Author

Commented:
THANKS FOR ALL YOUR HELP!!!!!

Commented:
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

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



 

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial