Problem with deck class.

This is a discussion on Problem with deck class. within the C++ Programming forums, part of the General Programming Boards category; [EDIT2!] The Library is now complete, unless I decide to change to a vector [/EDIT2!] [EDIT!] I found the error ...

  1. #1
    Devil's Advocate SlyMaelstrom's Avatar
    Join Date
    May 2004
    Location
    Out of scope
    Posts
    4,077

    Problem with deck class.

    [EDIT2!] The Library is now complete, unless I decide to change to a vector [/EDIT2!]
    [EDIT!] I found the error and I fixed it in this post... silly me forgot to do something on his to fix list and didn't fix the way I declared a new array here.

    I originally had DECK_SIZ being the number of cards, but I changed it to the number of decks and forgot to change my array from CardDeck[DECK_SIZ] to CardDeck[52 * DECK_SIZ]. You can look at this code if you wish for fun and reference and you can critique if you know what you're talking about, but I don't need it fixed anymore. Thanks for reading.

    Actually in truth I did find an error, but it's nothing I need help with. Just gotta fix some logic.[/EDIT!]

    Ok, I created a deck class that constucts a deck or multiple decks of your standard 52 card decks. I'm pretty new to classes and stacks, so there may be more errors in this than I know, but right now it compiles and runs but crashes when it attempts to output a card value. I don't know where the error is exactly in the class which is currently about 100 lines of actual code, so if you don't feel like picking through it, I don't blame you. If you are feeling kindly today, though...

    Here is the small main test I made:
    Code:
    #include <iostream>
    #include <iomanip>
    #include "CardDeck.h"
    
    int main() {
        Deck pCards(1);
        Card foo;
        for (int i = 0; i < 52; i++) {
            foo = pCards.Deal();
            std::cout << std::setw(2) << foo.faceVal << foo.Suit << std::setw(4) << foo.trueVal 
            << std::setw(20) << foo.fullName << std::endl;
            }
        std::cout << std::endl;
        pCards.Collect();
        pCards.Shuffle(5);
        for (int i = 0; i < 52; i++) {
            foo = pCards.Deal();
            std::cout << std::setw(2) << foo.faceVal << foo.Suit << std::setw(4) << foo.trueVal 
            << std::setw(20) << foo.fullName << std::endl;
            }
    
        std::cin.get();
        
        return 0;
    }
    ...and here is the Deck Class Library I made. Again I don't know exactly where the problem is so I have to post the whole thing. My guess is there is a problem with the calculations in the constructor, but then it don't have a problem declaring an instance of the class, so it gets through the constructor fine:
    Code:
    /* **************************************** */
    /* Deck of Cards                            */
    /* by William Nevin                         */
    /* 12/12/05 20:12 PM                        */
    /*        **************************        */
    /* Shuffle() - Iterates through the deck    */
    /*             echanging each card with a   */
    /*             randomly chosen card in the  */
    /*             deck.                        */
    /* Deal()    - Deals a single card off of   */
    /*             the top of the deck.         */
    /* Collect() - Resets the deck size to a    */
    /*             full deck.                   */
    /* Deck Size - Number of decks being used   */
    /* **************************************** */
    
    #ifndef StdCardDeckH
    #define StdCardDeckH
    
    #include <string>
    #include <sstream>
    #include <algorithm>
    
    const char HEART   = 0x03;
    const char DIAMOND = 0x04;
    const char CLUB    = 0x05;
    const char SPADE   = 0x06;
    
    struct Card {
        char Suit;
        int trueVal;
        std::string faceVal;
        std::string fullName;
    };
    
    class Deck
    {
      public:
        Deck(int DeckSize);
       ~Deck(void);
    
        void Shuffle(int passes);
        Card Deal(void);
        void Collect(void);
    
      private:
        Card *CardDeck;
        int CurrCard;
        const int DECK_SIZ;
    };
    
    Deck::Deck(int DeckSize = 1) : DECK_SIZ( DeckSize ) {
      std::ostringstream sConv;
      int CurrSuit, CurrDeck;
      
      CardDeck = new Card[52 * DECK_SIZ];
      for (CurrDeck = 0; CurrDeck < DECK_SIZ; CurrDeck++) {
        for (CurrSuit = 0; CurrSuit < 4; CurrSuit++) {
          for (CurrCard = 0; CurrCard < 13; CurrCard ++) {
            if ((CurrCard + 1) % 13 == 1) {
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].faceVal = 'A';
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].fullName = "Ace of ";
               }
            else if ((CurrCard + 1) % 13 == 11) {
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].faceVal = 'J';
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].fullName = "Jack of ";
               }
            else if ((CurrCard + 1) % 13 == 12) {
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].faceVal = 'Q';
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].fullName = "Queen of ";
               }
            else if ((CurrCard + 1) % 13 == 0) {
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].faceVal = 'K';
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].fullName = "King of ";
               }
            else {
               sConv << (CurrCard + 1) % 13;
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].faceVal = sConv.str();
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].fullName = sConv.str() + " of ";
               sConv.str("");
            }
            if (CurrSuit == 0) {
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].Suit = HEART;
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].fullName += "Hearts";
               }
            else if (CurrSuit == 1) {
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].Suit = DIAMOND;
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].fullName += "Diamonds";
               }
            else if (CurrSuit == 2) {
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].Suit = CLUB;
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].fullName += "Clubs";
               }
            else {
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].Suit = SPADE;
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].fullName += "Spades";
               }
            if ((CurrCard + 1) % 13 < 10 && (CurrCard + 1) % 13 != 0 )
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].trueVal = (CurrCard + 1) % 13;
            else
               CardDeck[CurrCard + (CurrSuit * 13) + (CurrDeck * 52)].trueVal =  10;
          }
        }
      }
      CurrCard = 0;
    }
    
    Deck::~Deck(void) {
      delete[] CardDeck;
    }
    
    void Deck::Shuffle(int passes = 1) {
      int currShuffle;
      for (currShuffle = 0; currShuffle < passes; currShuffle++)
          std::random_shuffle(CardDeck, CardDeck+DECK_SIZ*52);
    }
    
    Card Deck::Deal(void) {
      if ( CurrCard < DECK_SIZ * 52)
         return CardDeck[CurrCard++];
      else
         Collect();
         return CardDeck[CurrCard++];
    }
    
    void Deck::Collect(void) {
       CurrCard = 0;
    }
    
    #endif /* StdCardDeckH */
    What do you think? I haven't picked at it in it's current condition too much cause I don't want to screw it up more than it already is, so I figured there would be plenty of people here that can see where I'm having a problem.
    Last edited by SlyMaelstrom; 12-13-2005 at 07:09 PM.
    Sent from my iPadŽ

  2. #2
    Registered User
    Join Date
    Jan 2005
    Posts
    7,344
    I'd consider changing to a vector instead of the C style array.

    I'd also consider changing the include guard from __StdCardDeckH__. Words with double underscores are reserved, as are words that start with an underscore and an uppercase letter.

    I'm not sure what CurrSuit and CurrDeck are for. They look like they could be local to the constructor instead of members. If you plan to use them later, consider resetting them to a valid value at the end of your constructor (unless you want them to be invalid).

    I would not make member variables protected. Make them private instead. Your Deck class is not designed to be inherited (no virtual destructor) so no need for protected anyway.

    Also consider generating the card's string representations on the fly instead of storing them. This might take extra processing versus the memory saved, it is up to you on what makes more sense.

  3. #3
    Devil's Advocate SlyMaelstrom's Avatar
    Join Date
    May 2004
    Location
    Out of scope
    Posts
    4,077
    Thanks for the tips. I asked someone in the chatroom whether I could make variables local to the constuctor and they said no, that's why I made them members of the class. I was skeptic of their answer, but I decided to trust them.

    ...as for generating the string names of the cards on the fly, I could do that, but I wanted to design this for anyone to pick up and use in their program. The less they have to do the easier it is for them and I think calling fullName() is pretty easy.

    The rest of the tips are very nice. Again, thank you.

    Also, can I get a critique on this as far as portability and standardness goes? If this standard enough for anyone to basically pick up and use in their program on their compiler?
    Last edited by SlyMaelstrom; 12-13-2005 at 05:48 PM.
    Sent from my iPadŽ

  4. #4
    Registered User
    Join Date
    Jan 2005
    Posts
    7,344
    As far as I can tell it should be pretty portable. The only potential pitfall I see after quickly looking at the code is the suit characters (0x03, 0x04, etc). Those may not work on all platforms.

    Also, don't forget to make sConv local to the constructor.

    And I'll let you do the shuffle part, but remember it can be done in a single line with an algorithm even if you keep the C style array.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. problem in pass a variable to a class
    By nima_pw in forum C# Programming
    Replies: 3
    Last Post: 06-09-2009, 08:30 AM
  2. Mesh Class Design problem
    By sarah22 in forum Game Programming
    Replies: 2
    Last Post: 05-20-2009, 05:52 AM
  3. My Window Class
    By Epo in forum Game Programming
    Replies: 2
    Last Post: 07-10-2005, 03:33 PM
  4. Simple class problem
    By savageag in forum C++ Programming
    Replies: 1
    Last Post: 10-04-2003, 11:50 AM
  5. Replies: 3
    Last Post: 12-03-2001, 01:45 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21