Loops, looping more than they should.

I am working on this program to deal 13 random cards, and for some reason half the time one of my loops is looping more than they should... i cant work out why...

its the do loop in there...

Code:

---

#include <cstdlib>
#include <iostream>

using namespace std;

int main(int argc, char *argv[])
{
    start:
    system("cls");
   
    srand ( (unsigned)time ( NULL ) );
    int var[200];
    for (int i = 0; i < 199; i++ )
    var[i] = ( "&#37;d ", rand() % 100 );
   
   
    string getSpade[13] = {"A", "2", "3", "4", "5" ,"6", "7", "8", "9", "10", "J", "Q", "K"};
    random_shuffle(getSpade, getSpade + 13);
    string getHeart[13] = {"A", "2", "3", "4", "5" ,"6", "7", "8", "9", "10", "J", "Q", "K"};
    random_shuffle(getHeart, getHeart + 13);
    string getDiamond[13] = {"A", "2", "3", "4", "5" ,"6", "7", "8", "9", "10", "J", "Q", "K"};
    random_shuffle(getDiamond, getDiamond + 13);
    string getClub[13] = {"A", "2", "3", "4", "5" ,"6", "7", "8", "9", "10", "J", "Q", "K"};
    random_shuffle(getClub, getClub + 13);
   
   
    int nextSpade = 0, nextHeart = 0, nextDiamond = 0, nextClub = 0;
   
   
    string pOneSpade[13], pOneHeart[13], pOneDiamond[13], pOneClub[13];
   
    int f=0,i=0;
   
    do
    {
        if ((var[f]%2 != 0) && (nextSpade <= 12))
        {
            pOneSpade[nextSpade] = getSpade[nextSpade];
            nextSpade++; i++;
        }
        f++;
        if ((var[f]%2 != 0) && (nextHeart <= 12))
        {
            pOneHeart[nextHeart] = getHeart[nextHeart];
            nextHeart++; i++;
        }
        f++;
        if ((var[f]%2 != 0) && (nextDiamond <= 12))
        {
            pOneDiamond[nextDiamond] = getDiamond[nextDiamond];
            nextDiamond++; i++;
        }
        f++;
        if ((var[f]%2 != 0) && (nextClub <= 12))
        {
            pOneClub[nextClub] = getClub[nextClub];
            nextClub++; i++;
        }
        f++;
    } while (i <= 12);
   
    // Print player one's cards.
    cout << "Player One \n"
        << "S: ";
    for (int i = 0; i <= nextSpade-1; i++)
        cout << pOneSpade[i] << " ";
    cout << endl;
   
    cout << "H: ";
    for (int i = 0; i <= nextHeart-1; i++)
        cout << pOneHeart[i] << " ";
    cout << endl;
   
    cout << "D: ";
    for (int i = 0; i <= nextDiamond-1; i++)
        cout << pOneDiamond[i] << " ";
    cout << endl;
   
    cout << "C: ";
    for (int i = 0; i <= nextClub-1; i++)
        cout << pOneClub[i] << " ";
    cout << endl;
   
    // Play again option.
    char ex; 
    cout << "Play again? (y or n)";
    te:
    cin >> ex;
    switch (ex)
    {
          case 'y': goto start; break;
          case 'n': system("cls"); break;
          default: goto te; break;
    }
        return EXIT_SUCCESS;
}

---

ew... i see a goto...

Quote:

---

Code:

---

    do
    {
        if ((var[f]&#37;2 != 0) && (nextSpade <= 12))
        {
            pOneSpade[nextSpade] = getSpade[nextSpade];
            nextSpade++; i++;
        }
        f++;
        if ((var[f]%2 != 0) && (nextHeart <= 12))
        {
            pOneHeart[nextHeart] = getHeart[nextHeart];
            nextHeart++; i++;
        }
        f++;
        if ((var[f]%2 != 0) && (nextDiamond <= 12))
        {
            pOneDiamond[nextDiamond] = getDiamond[nextDiamond];
            nextDiamond++; i++;
        }
        f++;
        if ((var[f]%2 != 0) && (nextClub <= 12))
        {
            pOneClub[nextClub] = getClub[nextClub];
            nextClub++; i++;
        }
        f++;
    } while (i <= 12);

---

---

I have no idea what this code is supposed to do. I'm guessing it deals the cards, but only because of its location in the function.

Point is, the code is completely unreadable. Can you actually explain what it does, how it does it, and why it is superior to alternatives? I've a strong guess that a bug is hiding in there - with the i++ so split up, it's just waiting to blow up in your face - or perhaps it already did.

>> var[i] = ( "%d ", rand() % 100 );
What are you trying to do there? Simply using var[i] = rand() % 100; is probably what you want. The "%d" is ignored.

By the way, if you want a normal deck of cards and just 13 dealt from them, then your algorithm is not an accurate way to do that. The simplest and most common method is to place all 52 cards into your array and shuffle them. Then take the first 13 cards from the array (or however many you want).

You are storing your cards as strings, with four separate arrays for each suit. To fit them all in one array, I would store the suit with the rank: "S: A", "S: 2", ..., "C: K". That way you can store them all in one array and call random_shuffle once on that array, and then you'll just have to loop through the first 13 cards.

Sorry I wasn’t clearer with my original post, I actually worked out what was wrong with my loop shortly after I posted it then forgot all about my post….

Below I have posted part of the finished program to explain what I was trying to do there, I have added a few notes to help explain what I am doing… if you want to see the out put, it should compile perfectly…

I'm open on any suggestions on how to improve it, I am still new to c++ after a few days of ripping my hair out this was the bets I could come up with… :)

Code:

---

#include <cstdlib>
#include <iostream>

using namespace std;
int add_point(string);
int main(int argc, char *argv[])
{
    // This part of the code, builds a array of 199 random numbers
    srand ( (unsigned)time ( NULL ) );
    int var[200];
    for (int i = 0; i < 199; i++ )
    var[i] = ( rand() &#37; 100 );
   
    /*This part builds a deck of cards, I had to use a different array for each
    suit so that I would deal the same card twice. And I also had to use a
    string because if I used a char it wouldn't display "10" because it is 2 characters… I think*/
    string getSpade[13] = {"A", "2", "3", "4", "5" ,"6", "7", "8", "9", "10", "J", "Q", "K"};
    random_shuffle(getSpade, getSpade + 13);
    string getHeart[13] = {"A", "2", "3", "4", "5" ,"6", "7", "8", "9", "10", "J", "Q", "K"};
    random_shuffle(getHeart, getHeart + 13);
    string getDiamond[13] = {"A", "2", "3", "4", "5" ,"6", "7", "8", "9", "10", "J", "Q", "K"};
    random_shuffle(getDiamond, getDiamond + 13);
    string getClub[13] = {"A", "2", "3", "4", "5" ,"6", "7", "8", "9", "10", "J", "Q", "K"};
    random_shuffle(getClub, getClub + 13);
   
    //These arrays are there to count the number of cards from each suit a player would get.
    int nextSpade = 0, nextHeart = 0, nextDiamond = 0, nextClub = 0;
   
    int f = 0, i = 0, spp1 = 0, hep1 = 0, dip1 = 0, clp1 = 0;
   
    /*This loop was the most random way I could get the program to deal cards.
    First it check the value of "var[f]" to see if it is odd or even. if that true
    Then it checks to see if all the spades have been dealt, if they haven't it then
    check to see if that player hasn't all ready received 13. (this program is a bridge
    card dealer, 13 cards per player 4 players) then finally if all the values are met,
    it adds one to nextSpade or nextHeat etc etc*/
    do
    {
        if ((var[f]%2 != 0) && (nextSpade <= 12) && (i <= 12))
        { nextSpade++; i++; spp1++; } f++;
       
        if ((var[f]%2 != 0) && (nextHeart <= 12)&& (i <= 12))
        { nextHeart++; i++; hep1++; } f++;
       
        if ((var[f]%2 != 0) && (nextDiamond <= 12)&& (i <= 12))
        { nextDiamond++; i++; dip1++; } f++;
       
        if ((var[f]%2 != 0) && (nextClub <= 12)&& (i <= 12))
        { nextClub++; i++; clp1++; } f++;
       
    } while (i <= 12);
   
    /*This section of code is the display part, it prints the first card,
    then increases i, display the next card etc etc and will loop until
    i = the number of cards that player is meant to get for that suit*/
    cout << "Player One \n"
        << "S: ";
    for (int i = 0; i <= nextSpade-1; i++)
        cout << getSpade[i] << " ";
    cout << endl;
   
    cout << "H: ";
    for (int i = 0; i <= nextHeart-1; i++)
        cout << getHeart[i] << " ";
    cout << endl;
   
    cout << "D: ";
    for (int i = 0; i <= nextDiamond-1; i++)
        cout << getDiamond[i] << " ";
    cout << endl;
   
    cout << "C: ";
    for (int i = 0; i <= nextClub-1; i++)
        cout << getClub[i] << " ";
    cout << endl;
   
    i=0; int spp2 = spp1, hep2 = hep1, dip2 = dip1, clp2 = clp1;
    // same a previous loop for player one
    do
    {
        if ((var[f]%2 != 0) && (nextSpade <= 12) && (i <= 12))
        { nextSpade++; i++; spp2++; }
        f++;
        if ((var[f]%2 != 0) && (nextHeart <= 12) && (i <= 12))
        { nextHeart++; i++; hep2++; }
        f++;
        if ((var[f]%2 != 0) && (nextDiamond <= 12) && (i <= 12))
        { nextDiamond++; i++; dip2++; }
        f++;
        if ((var[f]%2 != 0) && (nextClub <= 12) && (i <= 12))
        { nextClub++; i++; clp2++; }
        f++;
    } while (i <= 12);
    // just the same as loop from player one, but this one i = spp1, which is the number
    // of cards of that suit that the previous player was dealt...
    cout << "Player Two \n"
        << "S: ";
    for (int i = spp1; i <= nextSpade-1; i++)
        cout << getSpade[i] << " ";
    cout << endl;
   
    cout << "H: ";
    for (int i = hep1; i <= nextHeart-1; i++)
        cout << getHeart[i] << " ";
    cout << endl;
   
    cout << "D: ";
    for (int i = dip1; i <= nextDiamond-1; i++)
        cout << getDiamond[i] << " ";
    cout << endl;
   
    cout << "C: ";
    for (int i = clp1; i <= nextClub-1; i++)
        cout << getClub[i] << " ";
    cout << endl;
   
    system("pause");
    return EXIT_SUCCESS;
}

---

Quote:

---

Originally Posted by Daved

>> var[i] = ( "%d ", rand() % 100 );
What are you trying to do there? Simply using var[i] = rand() % 100; is probably what you want. The "%d" is ignored.

By the way, if you want a normal deck of cards and just 13 dealt from them, then your algorithm is not an accurate way to do that. The simplest and most common method is to place all 52 cards into your array and shuffle them. Then take the first 13 cards from the array (or however many you want).

You are storing your cards as strings, with four separate arrays for each suit. To fit them all in one array, I would store the suit with the rank: "S: A", "S: 2", ..., "C: K". That way you can store them all in one array and call random_shuffle once on that array, and then you'll just have to loop through the first 13 cards.

---

That wouldn't work with how i want the out put to be..

Quote:

---

Player One
S: 6 2
H: 5 7 A
D: 8 6 J Q A
C: 7 10 9
11 Points
Player Two
S: 5 J 9 K
H: 8 J 2 3
D: 10 4 2
C: 8 2
5 Points
Player Three
S: Q 3 8 A 10
H: 4 10 Q
D: 3 5 9 K
C: Q
13 Points
Player Four
S: 7 4
H: K 6 9
D: 7
C: 5 J 3 K 4 A 6
11 Points

Deal again? (y or n)

---

I didn't go through your algorithm carefully, but I would be worried that it does not provide an even distribution. I understand that you want your display to separate based on suits, but the most important thing is to make sure you get a random shuffle of cards.

The tried and true and tested way to shuffle cards is to put everything into a single array and call random_shuffle. Coming up with a new method is possible, but it is very easy to get a not quite random distribution. What you might end up getting is that one player wins a lot more often than another.

I would consider using a single array and just trying to come up with away to solve the display problem. One possibility is to store a struct (if you know classes/structs) which holds the suit and the rank separately. Another possibility is to use numbers from 1-52 (or 0-51) and just convert the number to the proper rank and suit in a separate function. How you do it depends on how much C++ you're familiar with.

The way I always do playing-card-related stuff, is to see each card as a number between 0 and 51, see the rank as a number between 0 and 12, and the suit as a number between 0 and 3. That gives me:

rank = cardnumber % 13
suit = cardnumber / 13

So I could then use something like

Code:

---

const char* rankchars = "23456789TJQKA";
const char* suitchars = "shdc";

---

...or whatever, for the output.

To do what the program above does, or tries to do, I would store the cards in a std::vector<int> (containing the ints 0, 1, ..., 51) , deal them out by using pop_back() (and pop()) to four other std::vector<int>... or even better, an array of std::vector<int> (representing the players' hands). I'd then sort the hands according to the suits. Then I'd loop through each player's hand, output one card at the time, checking which suit it has and adjusting the output accordingly.

I could post some code, but first I don't know if this is homework (you never know around here ;)), second I don't know if you'd get what the heck was going on if you're as new to the language as you say you are.