Loops, looping more than they should.

This is a discussion on Loops, looping more than they should. within the C++ Programming forums, part of the General Programming Boards category; I am working on this program to deal 13 random cards, and for some reason half the time one of ...

  1. #1
    Registered Abuser Loic's Avatar
    Join Date
    Mar 2007
    Location
    Sydney
    Posts
    115

    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;
    }
    Last edited by Loic; 04-30-2007 at 06:23 PM.

  2. #2
    Registered User
    Join Date
    Dec 2005
    Location
    Canada
    Posts
    267
    ew... i see a goto...

    OS: Windows 7, XUbuntu 11.10, Arch Linux
    IDE: CodeBlocks
    Compiler: GCC

  3. #3
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    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.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  4. #4
    Registered User
    Join Date
    Jan 2005
    Posts
    7,317
    >> var[i] = ( "&#37;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.

  5. #5
    Registered Abuser Loic's Avatar
    Join Date
    Mar 2007
    Location
    Sydney
    Posts
    115
    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;
    }

  6. #6
    Registered Abuser Loic's Avatar
    Join Date
    Mar 2007
    Location
    Sydney
    Posts
    115
    Quote Originally Posted by Daved View Post
    >> 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..

    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)

  7. #7
    Registered User
    Join Date
    Jan 2005
    Posts
    7,317
    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.

  8. #8
    Registered User
    Join Date
    Apr 2007
    Location
    Sweden
    Posts
    41
    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 &#37; 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.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Too many loops D:
    By F5 Tornado in forum C++ Programming
    Replies: 6
    Last Post: 12-03-2007, 12:18 AM
  2. strings and loops...
    By twomers in forum C Programming
    Replies: 5
    Last Post: 12-12-2005, 10:28 AM
  3. Newbie in problem with looping
    By nrain in forum C Programming
    Replies: 6
    Last Post: 11-05-2005, 11:53 AM
  4. Looping questions
    By Peyote in forum C++ Programming
    Replies: 3
    Last Post: 09-15-2003, 11:01 PM
  5. help with arrays and loops
    By jdiazj1 in forum C Programming
    Replies: 4
    Last Post: 11-24-2001, 03:28 PM

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