views:

123

answers:

3

Hi I am working on a program where I have to initialize a deck of cards. I am using a struct to represent a card. However I'm not filling it correctly as I get a bunch of zero's when I display the deck of cards. I believe my mistake is in this line but I'm not sure:

struct card temp = {"Clubs", value, false};

The code:

void initCards(){

    int count = 0;
    int location = 0;
    const int hand = 12;

    //add hearts
    int value=2;
    while( count < hand ){
        struct card temp = {"Hearts", value, false};
        cards[location] = temp;
        value++;
        count++;
    }

    count = 0;

    //add diamonts
    value = 2;
    while( count < hand ){
        struct card temp = {"Diamonds", value, false};
        cards[count] = temp;
        value++;
        count++;
    }

    //add spades
    count = 0;
    value = 2;
    while( count < hand ){
        struct card temp = {"Spades", value, false};
        cards[count] = temp;
        value++;
        count++;
    }

    //add clubs
    count = 0;
    value = 2;
    while( count < hand ){
        struct card temp = {"Clubs", value, false};
        cards[count] = temp;
        value++;
        count++;
    }

    //print the deck
    for(int i=0; i<52; i++){
        cout << cards[i].type << " " << cards[i].rank << endl;
    }
}

I can't believe I was using count as my iterator... location was what I intended to use. And since I started count at 2, hand should be 13. Sometimes you just need to take a break and come back to catch the errors. This works fine:

void initCards(){

    int count = 0;
    int location = 0;
    const int hand = 13;

    //add hearts
    int value=2;
    while( count < hand ){
        struct card temp = {"Hearts", value, false};
        cards[location] = temp;
        value++;
        location++;
        count++;
    }

    count = 0;

    //add diamonts
    value = 2;
    while( count < hand ){
        struct card temp = {"Diamonds", value, false};
        cards[location] = temp;
        value++;
        location++;
        count++;
    }

    //add spades
    count = 0;
    value = 2;
    while( count < hand ){
        struct card temp = {"Spades", value, false};
        cards[location] = temp;
        value++;
        location++;
        count++;
    }

    //add clubs
    count = 0;
    value = 2;
    while( count < hand ){
        struct card temp = {"Clubs", value, false};
        cards[location] = temp;
        value++;
        location++;
        count++;
    }


    for(int i=0; i<52; i++){
        cout << cards[i].type << " " << cards[i].rank << endl;
    }
}
+3  A: 

You reset count to zero every time you add a new suit. So, presuming cards is big enough to hold 52 elements, most of them aren't going to be filled because you keep writing over ones at the beginning.

If you post the declarations of struct card and cards we'll better be able to help you.

Kristo
+1 for asking for more information.
Thomas Matthews
A: 

The array of cards is always overwritten by the last card type. You should not reset count to 0, instead you should check in the while condition if it is less than its value + 12.

int temp = count + 12;
while(count < temp)
{
}

Additionally I would reccommend you to hold your cards in a pointer array. This will make you save a lot of copy operations, as in this case every time your a assinging a card to an array index you are copying the entire structure object.

Simon
Don't use pointers for small data-holding objects unless you have found it the copy operations to be a hotspot. The copying is most likely negligible in this case.
Georg Fritzsche
+1  A: 

Here's some code:

struct Card
{
  virtual const std::string& get_suite_name(void) const = 0;
  unsigned int value;
};

struct Spade_Card
: public Card
{
    const std::string& get_suite_name(void) const
    {
        static const std::string name = "Spades";
        return name;
    }
};

struct Heart_Card
: public Card
{
    const std::string& get_suite_name(void) const
    {
        static const std::string name = "Hearts";
        return name;
    }
};

struct Clubs_Card
: public Card
{
    const std::string& get_suite_name(void) const
    {
        static const std::string name = "Clubs";
        return name;
    }
};

#define CARDS_IN_SUITE 13
#define NUM_SUITES 4
#define CARDS_IN_DECK ((CARDS_IN_SUITE) * (NUM_SUITES))

int main(void)
{
    std::vector<Card *> deck;
    // Create the hearts suite & add to the deck.
    unsigned int i = 0;
    for (i = 0; i < CARDS_IN_SUITE; ++i)
    {
        Card * p_card = new Spade_Card;
        if (!p_card)
        {
            cerr << "Error allocating memory for a card." << endl;
            return EXIT_FAILURE;
        }
        p_card->value = i + 1;
        deck.push_back(p_card);
    }

    // Repeat for other suites.

    for (i = 0; i < CARDS_IN_DECK; ++i)
    {
         delete deck[i];  // Good karma to deallocate memory.
    }
    return EXIT_SUCCESS;
}

The suite name doesn't need to be repeated for every card. A card shares a suite name with 12 other cards. A card has-a suite name.

Perhaps this is taking OO a bit too far. ;-)

Thomas Matthews
What a mess... Inheritance...abuse of the concepts present in OOD... My eyes are on fire! Also, you can automate the creation of suits...if you don't rely on using inheritance the way you did. A simple local array of strings filled with the names "Hearts", "Diamonds", "Clubs" and "Spades" will do nicely with such a loop. Of course, keeping it unrolled is a better idea for efficiency, but given your code, I doubt efficiency was kept in mind. :P +1 for such a creative answer, though! :D
Dustin