# A real puzzling problem

## Recommended Posts

##### Share on other sites
My first gripe is that your "shuffled" deck doesn't look very shuffled. My second gripe is that you are using hungarian notation ;-)

I see one potential problem: you are using a static variable in your TakeCard() function. I'm not totally sure, but i think that one variable is good for the entire class. But i think that's what your intention actually was. This is bad. Basically you are making the PLAYER class responsible for getting the right cards delt. This is obviously a responsibility of the DECK class. I would think that the better way to do it would be like this (unoptimised and overly simplified, uses STL):

class Deck {   public:      Deck() {         // create deck and shuffle         for (int i=0; i < 52; i++) {            // add each card to the vector (code omitted for bevity)            }            // shuffle deck the STL way ;-)            random_shuffle( cards.begin(), cards.end() );         }      Card Deck::TakeCard() {         return cards.pop_back();      }   private:      vector<Card> cards;   }

Then you can let the deck worry about what cards it hands out. That should be out of the Player class code, for sure. After that, you can do the regular dealing hokus pokus

for(int i=0; i < num_players; i++) {   for(int j=0; j < num_cards; j++) {      players[i].AcceptCard( deck.TakeCard() );      }   }

I'm assuming you already know STL in the previous example. If that's assuming too much, now is a good time to get acquainted :-) The important part is that you put the responsibilities in the right classes. The deck distributes the cards. Players only receive cards.

EDIT:

but more to the point, you could probably move your static "n" variable into the deck's dealCard() function and out of the player's function.

##### Share on other sites
The cards do shuffle just fine; I just wanted to give a simple example of how the cards were being handled. I am familiar with the STL, just wanted to do some things from scratch for the heck of it. As for the hungarian notation, well I guess it is just a style that works for me. What about Bill Gates notation where the prefix for any variable and function would be a '\$'. ;)

Well I moved the static variable to the deck class. But that doesn't really change the fact the my hand array is retaining the previous card issued to the previous player. When I have declared an array of objects (players) shouldn't each player's hand be intialized by the class constructor? Also how is the value being retained in this function? Doesn't it lose scope upon exiting the function? Let me show you my updated code first.
----------------------------------------------------------
//inside the driver
cPlayer *people; //the number of people playing

cDeck testdeck(DECKSIZE); //the new deck
cout << "\n how many people are playing? ";
cin >> x;

people = new cPlayer[x]; //array of players

for(int j=0; j < x; j++)
for(int t = 0; t < INITIALHAND; t++)
people[j].TakeCard(t,&testdeck); //deal out 7 cards
----------------------------------------------------------
void cPlayer::TakeCard(int iNumberOfCards,cDeck *deck){

hand[iNumberOfCards] = deck->dealCard(); //player gets card from deck
}
-------------------------------------------------------

cCard cDeck::dealCard(){
static int icardCounter = -1; //return the card off the top of the deck
icardCounter++;
return myDeck[icardCounter];
}

---------------------------------------------------------

The results are the same. Any new ideas?

##### Share on other sites
Looks to me like a possible case of going out of the boundaries of the array... Are you sure that when you create a new cPlayer, that it's 'hand' array gets created correctly with size INITIALHAND?

Vovan

##### Share on other sites
Well here is my constructor for the player class. I created an array of players so this constructor gets called however many players are needed.

cPlayer::cPlayer(){
iScore = 0;
iPlayerID = 0;
hand = new cCard(INITIALHAND); //size of starting hand initial hand is 7 cards
}

As for the cCard constructor, all it does it set up values for the cards. I have eliminated the cCard class as the problem. I think the problem lies somewhere in the player class.

##### Share on other sites
So... What's the type of 'hand'? I mean, it looks to me from initialization like it's a single card, and yet you access it as an array in TakeCard. To make it an array, they should be square brackets around INITIALHAND, just like when you are making the players array in your main function... Right?

Vovan

##### Share on other sites
Notice in the earlier code posts the code snippet:

void cPlayer::TakeCard(int iNumberOfCards,cDeck *deck){

hand[iNumberOfCards] = deck->dealCard();
}

This method has access to hand so hand has to be a member of cPlayer. Hand is of course private because to be public would be poor designing. So hand is a pointer to a cPlayer array.

##### Share on other sites
Quote:
 Original post by Captain DuhSo hand is a pointer to a cPlayer array.

OK, so now we are definitely getting confused here. ;)

First of all, it holds cards, and not players. Correct?

Secondly, you initialize it to be a single card, not an array. So, how does that work?

Vovan

##### Share on other sites
I think you may be on to something. I think the there may be a screw up in the one area I was certain was not the problem - the card constructor. It looks like this:

cCard::cCard(int ihandSize)
{ //creates a new hand for a player
chFace = NULL;
chColor = NULL;
iValue = NULL;
iID = NULL;
}

So then when I call this cPlayer constructor method (below), the hand member is actually only pointing to a single card element per player isn't it?

cPlayer::cPlayer(){
iScore = 0;
iPlayerID = 0;
hand = new cCard(INITIALHAND); //size of starting hand initial hand is 7 cards
}

So when I get into the TakeCard method, the array is being written outside it's bounds. Because there is no bounds checking, I get a majot access violation with my array.

void cPlayer::TakeCard(int iNumberOfCards,cDeck *deck){

hand[iNumberOfCards] = deck->dealCard();
}

This might explain the exceptions I get in the debugger:

First-chance exception at 0x7c92a653 in SparkyProject.exe: 0xC0000005: Access violation reading location 0x0000000c.
First-chance exception at 0x7c910f29 in SparkyProject.exe: 0xC0000005: Access violation reading location 0xdddddddd.
First-chance exception at 0x7c910f29 in SparkyProject.exe: 0xC0000005: Access violation reading location 0xdddddddd.

These exceptions are something that I haven't been able to track down as of yet. Tee hee. I think you are on to something here. Let me redesign that card constructor and I will reply after that - unless you see something else. Thanks.

##### Share on other sites
That was it! Damn, was a major oversite on my part. That was also the cause of the access violations. That was something that I should have seen a long time ago. Thanks a lot man. I appreciate it, now I can go to sleep with a clear mind. :D.

##### Share on other sites
Great. [smile] Glad you got it working.

Vovan

## Create an account

Register a new account

• ## Partner Spotlight

• ### Forum Statistics

• Total Topics
627655
• Total Posts
2978460

• 10
• 12
• 22
• 13
• 33