coin flip problem

Started by
29 comments, last by rip-off 10 years, 7 months ago

If you want to get a little adventurous, try running your debugger. I assume you have one available. Step your code and see the values it takes at each step.

Advertisement

Could you post the current state of your code?


#include <ctime>
#include <cstdlib>
#include <iostream>
#include <string>

using namespace std;


int choice;
int randRange (int low, int high)

{
        return rand() % ( high - low ) + low;
    }

int main ()
{

 cout << "flip a coin? \n\n1: Yes\n\n2: No\n";
cin >> choice;

 while (!(choice == 1 || choice == 2))
 {
        cin >> choice;
        cout << "please enter 1 or 2";
 }
 if (choice == 1)
    {
            srand ( time( NULL ) );
            randRange( 1 , 10 );
            if (randRange( 1,10) >= 5)
            {
                cout << "Heads";
            }
else
            {
            cout << "Tails";
            }
    }
else if (choice == 2)
            {
            cout << "Boooo";
            return 0;
            }

}

Bugs:

- If people dont type in the right number you print out what to do only after they should have typed it in again, would be better the other way around.

- You uselessly call your randRange function twice without using the result of the first call.

Possible improvements:

- You can move the choice variable into main(), it doesnt need to be a global variable.

- Please try to consistently indent everything, as this makes reading the code easier. For example the "else" should be on same level as the "if" and all statements inside a block on same indendation level.

- If you want to modify it later with a loop to throw more than one coin its safer to put the srand call at the beginning of main so you dont accidently call it more than once. Calling it more than once would make the quality of random numbers worse.

As SiCrane and Washu pointed out, you need to check if 'cin' failed.

When you do:
int myInt = 0;
std::cin >> myInt;
std::cin wants to give 'myInt' a number, because ints can only hold numbers. But if the user types in something that isn't a number, like a letter or a period, std::cin needs some way to inform your code that something went wrong. To do this, std::cin has several 'flags' that are set on it: whether it failed, whether it ran out of input, or whether something else went wrong. [documentation]

Instead of checking each flag, to make sure nothing went wrong, std::cin, for convenience, also has a customized special use of the '!' operator. If (!cin) is true, that means something went wrong.

This code works:
#include <ctime>
#include <cstdlib>
#include <iostream>
#include <string>

using namespace std;

//Don't declare variables outside of functions (these are called 'globals'), it's a bad practice
//that while making things easier earlier on in programming, actually causes problems with more
//advanced programming.
//int choice;

int randRange (int low, int high)
{
    return rand() % ( high - low ) + low;
}

int main ()
{
    //srand() only needs to be called once for your entire program, so it's best to just
    //do so near the program start.
    srand ( time( NULL ) );
    
    cout << "flip a coin?\n1: Yes\n2: No\n";
    
    //Declare variables as close as possible to when you first use them, and also initialize 
    //them to a good default value - this makes it easier to detect bugs when they occur.
    int choice = 0; 
    cin >> choice;
    
    while (!(choice == 1 || choice == 2) )
    {
        //Incase of problems getting input, we check for if something went wrong with 'cin'.
        //'cin' has a special use of the '!' operator built in, that when used on 'cin',
        //lets you know if something went wrong.
        if(!cin)
        {
            std::cout << "Invalid input was given - it wasn't a number." << std::endl;
            
            //Since 'cin' was marked with a flag that said it got invalid input,
            //we need to clear that flag to let 'cin' know we saw and responded to the problem.
            cin.clear();
        }
        
        cin >> choice;
        cout << "please enter 1 or 2";
    }

    if (choice == 1)
    {
        randRange( 1 , 10 );
        if (randRange( 1,10) >= 5)
        {
            cout << "Heads";
        }
        else
        {
            cout << "Tails";
        }
    }
    else if (choice == 2)
    {
        cout << "Boooo";
        return 0;
    }
}

[Edit:] The OP's code indentation issues come from mixing spaces and tabs, and then the tabs in his IDE being a different number of spaces from the tabs in GameDev.net's code box. Happens to me alot too! I wish IDEs automaticly converted spaces to tabs when you copied text (I think my IDE only does that on file save) - but that'd probably cause other problems. =)

I was going to say the same, cout should be before the cin, it looks like the program is in an infinite loop but it isn't.

Also, some suggestions about the rest of the code:

  • You should change the ">= 5" for either a ">= 6" or "a > 5". With ">= 5" you're taking 1, 2, 3 or 4 for "tails" and 5, 6, 7, 8, 9 and 10 for "Heads".
  • Also, you can change it for something like "if (randRange(0,1) == 0)", it looks much clearer that there's a 50-50 chance for each value.
  • If you checked that after the "while" the variable "choice" can be only a 1 or a 2, you can write if (choice == 1) { ... flip the coin ... } else { cout << "booo" }, you don't have to check again, if it's not a 1 then it can only be a 2. If you need to check the same variable for different values it's better to use a switch statement instead.
  • The line "return 0;" shouldn't be there, it's only being executed if you typed "2". It should be only at the end of the main function if you'll always return a 0.

As SiCrane and Washu pointed out, you need to check if 'cin' failed.

When you do:


int myInt = 0;
std::cin >> myInt;
std::cin wants to give 'myInt' a number, because ints can only hold numbers. But if the user types in something that isn't a number, like a letter or a period, std::cin needs some way to inform your code that something went wrong. To do this, std::cin has several 'flags' that are set on it: whether it failed, whether it ran out of input, or whether something else went wrong. [documentation]

Instead of checking each flag, to make sure nothing went wrong, std::cin, for convenience, also has a customized special use of the '!' operator. If (!cin) is true, that means something went wrong.

This code works:

#include <ctime>
#include <cstdlib>
#include <iostream>
#include <string>

using namespace std;

//Don't declare variables outside of functions (these are called 'globals'), it's a bad practice
//that while making things easier earlier on in programming, actually causes problems with more
//advanced programming.
//int choice;

int randRange (int low, int high)
{
    return rand() % ( high - low ) + low;
}

int main ()
{
    //srand() only needs to be called once for your entire program, so it's best to just
    //do so near the program start.
    srand ( time( NULL ) );
    
    cout << "flip a coin?\n1: Yes\n2: No\n";
    
    //Declare variables as close as possible to when you first use them, and also initialize 
    //them to a good default value - this makes it easier to detect bugs when they occur.
    int choice = 0; 
    cin >> choice;
    
    while (!(choice == 1 || choice == 2) )
    {
        //Incase of problems getting input, we check for if something went wrong with 'cin'.
        //'cin' has a special use of the '!' operator built in, that when used on 'cin',
        //lets you know if something went wrong.
        if(!cin)
        {
            std::cout << "Invalid input was given - it wasn't a number." << std::endl;
            
            //Since 'cin' was marked with a flag that said it got invalid input,
            //we need to clear that flag to let 'cin' know we saw and responded to the problem.
            cin.clear();
        }
        
        cin >> choice;
        cout << "please enter 1 or 2";
    }

    if (choice == 1)
    {
        randRange( 1 , 10 );
        if (randRange( 1,10) >= 5)
        {
            cout << "Heads";
        }
        else
        {
            cout << "Tails";
        }
    }
    else if (choice == 2)
    {
        cout << "Boooo";
        return 0;
    }
}
[Edit:] The OP's code indentation issues come from mixing spaces and tabs, and then the tabs in his IDE being a different number of spaces from the tabs in GameDev.net's code box. Happens to me alot too! I wish IDEs automaticly converted spaces to tabs when you copied text (I think my IDE only does that on file save) - but that'd probably cause other problems. =)

You missed a cin.ignore, clear is not sufficient.

In time the project grows, the ignorance of its devs it shows, with many a convoluted function, it plunges into deep compunction, the price of failure is high, Washu's mirth is nigh.


cin.ignore(std::numeric_limits<streamsize>::max());
cin.clear();

Is that correct?

No, you need to reverse the order. Clear the error flag, then empty the buffer That will ignore upto std::char_traits<T>::eof() from the current stream, and put it back in a position to be read from safely.

In time the project grows, the ignorance of its devs it shows, with many a convoluted function, it plunges into deep compunction, the price of failure is high, Washu's mirth is nigh.

Also, although the side effect of using int modulo is fairly small, you should note that rand()[0..2147483647] % x

Yields a bias to your results, as the max value returned by rand() is often not evenly divisible by x.

Using cin.ignore(std::numeric_limits<streamsize>::max()); is highly inadvisable. This will not only flush the buffer of the current contents, but also tells the stream to sit there and eat up all future contents until EOF is hit. If you're going to use an EOF delimiter then set the number of ignored characters to something reasonable such as the number of characters still in the buffer. Otherwise if you're going to use std::numeric_limits<std::streamsize>::max() as the number of characters set the delimiter to something the user will actually put into the stream like '\n'.

This topic is closed to new replies.

Advertisement