Safely prompt for yes/no with cin

Question

I'm in an intro to C++ class and I was wondering of a better method of checking if input was the desired type.

Is this a good way of doing this? I come from a PHP/PERL background which makes me rather apprehensive of using while loops.

char type;
while (true) {
    cout << "Were you admitted? [y/n]" << endl;
    cin >> type;

    if ((type == 'y') || (type == 'n')) {
        break;
    }
}

Is this a safe way of doing this or am I opening myself up to a world of hurt, which I suspect? What would be a better way of making sure I get the input I want before continuing?

Spaulding

What's the significance of the number 10 in this situation? – Jeanne

There is no real significance, I meant it as saying there was a limit to how long the loop would run. But even there lies a risk of what if i never increments. – Spaulding

I guess the title really should be while( condition ) vs while(true) { if( condition ) break. – Dilorenzo

Changed title; thanks for the input! – Spaulding

"I come from a PHP/PERL background which makes me rather apprehensive of using while loops" - Both PHP and Perl have while loops... – Whipple

I know they have while loops, many of my teachers would teach us not to use them because they worried we would crash servers. Definitely not the best way to learn to program though. – Spaulding

@BlueRaja: range-based loops with foreach and the like replaced many uses of for and while in dynamic languages. that has had the nice effect of squashing many bugs in the terminating logic, and the ugly effect that people now feel insecure when they need a while. but that's only once in a while, for sure! – Ahasuerus

@just: there isn't really a foreach replacement for while(type != 'n' && type != 'y') – Whipple

@BlueRaja I wrote "foreach [...] replaced many uses of for and while", notice many as opposed to all. My claim that "poeple now feel insecure when they need a while [statement]" should also make it clear that while is still needed. But maybe I'm just conflating dynamic languages with "enterprise" (web) applications where vast majority of loops are over collections such as db resultsets. For example a banking application I've been working on contains 80 while loops and 1590 foreach loops. – Ahasuerus

Answer 1

Personally I'd go with:

do
{
    cout << "Were you admitted? [y/n]" << endl;
    cin >> type;
}
while( !cin.fail() && type!='y' && type!='n' );

Stretto

This isn't good. If cin gets closed it will loop forever spewing "Were you admitted? [y/n]" to cout. It's important to always test the success of the input operation before testing what it's supposed to have read. – Coccidiosis

Much more readable and concise than my code! Are Do-Whiles what is commonly used for make sure input is correct and if not to ask again? – Spaulding

Do-Whiles are mostly used in cases that you want to take some action before checking the condition, and this action would be repeated if your condition is true. In case of user input, before you check the condition you want to get some input, so Do-Whiles are convenient. – Pettway

Do-Whiles are generally used whenever you want the code to execute at least once. – Jeanne

Edited per Charles Bailey's comment - he's exactly right. And Alex's comment answers Levi's comment. You could use a normal while loop in this case if you initialized type to something other than y or n. – Stretto

Is it any better? If cin fails it still loops forever. – Coccidiosis

@Stretto The condition should be (!cin.fail() && type!='y' && type!='n'); – Pettway

@ Charles - forgot some of the intricacies of using cin, thanks. @Alex - How's that any different? – Stretto

@Stretto If cin fails, you don't want to loop forever. In your implementation, if cin fails, the condition is still true because type is neither 'y' nor 'n', so the loop continues. Instead, you want to ensure that if cin fails the condition becomes false, thus ( !cin.fail() && type!='y' && type!='n' ) – Pettway

If cin really has been closed then calling clear() may be futile. The next read attempt would immediately re-set the fail bit and the infinite loop writing to cout remains. – Coccidiosis

Misread and wasn't actually thinking about a CLOSED stream. I missed that in Charles' comments. Charles and Alex are both correct. – Stretto

The clear/ignore is, of course, wrong depending on what input was last performed (before this loop). You should always read a full line when doing input (if doing line-based input) rather than randomly leaving a newline or not. !cin.fail() is the same as cin. – Gayden

24 upvotes, 500 people discussing a problem that comes right after 'hello world', and the end result is something that will immediately hang on the ignore and not even begin to work. I don't know which is more pathetic, C++ iostreams or that people will argue over the finest of details but never bother to run the damn thing. – Lay

Answer 2

Personally I'd make the prompt a separate function, this makes it putting the prompt output and reading a response a logical expression to put in a while loop.

Testing whether the read was successful is critical to the correct functioning of the code.

I'd also prefer to use std::getline to get a line at a time as it helps reduce errors caused by reading the rest of a half read line that was the result of a partial read to earlier user responses.

bool PromptForChar( const char* prompt, char& readch )
{
    std::string tmp;
    std::cout << prompt << std::endl;
    if (std::getline(std::cin, tmp))
    {
        // Only accept single character input
        if (tmp.length() == 1)
        {
            readch = tmp[0];
        }
        else
        {
            // For most input, char zero is an appropriate sentinel
            readch = '\0';
        }
        return true;
    }
    return false;
}

void f()
{
    char type = '\0';

    while( PromptForChar( "Were you admitted? [y/n]", type ) )
    {
        if (type == 'y' || type == 'n')
        {
            // Process response
            break;
        }
    }
}

Coccidiosis

+1. Use a for loop to scope the variable. Returning the stream is more versatile and still allows testing as if it was a bool. – Gayden

Which variable should be scoped? The type? – Fernferna

Answer 3

Use can use

do {
    program;
} while (condition_to_repeat);

if the algorithm is similar to your example. Otherwise the example is "safe", but I am not sure about the readablity.

Fleeman

Answer 4

Why not do it this way?

do
{
    cout << "Were you admitted? [y/n]" << endl;
    cin >> type;
}while(  type !='y' && type !='n');

Majestic

I considered the target... this is an intro to C++ class. It is assumed in these early classes that cin is connected to the command-line/OS, and that it doesn't fail. – Majestic

^Z in Windows or ^D in Unix will cause the command-line cin to return EOF the first time and FAIL the second time. – Chloroform

@jmucchiello: It will set failbit the first time when it reaches EOF trying to read the char. (The char cannot be read, which must be read, so the stream has 'failed'.) – Gayden

Okay. Regardless, it still will never exit. – Chloroform

Answer 5

That's fine. If you want it to time out after a number of failures, you could use the i<10 but its not worth it.

Haemolysis

Answer 6

And don't forget to make your potential user's life easier explaining each step and even provide the case insensitive input.

#include <iostream>

#define MAX_USER_INPUT_ATTEMPTS 3

int _tmain(int argc, _TCHAR* argv[])
{
char input_value = ' ';
int current_attempt = 1;

while(true)
{
    std::cout << "Please confirm your choice (press y[es] or n[o] and Enter): ";

    std::cin >> input_value;

    input_value = tolower( input_value );

    if(input_value=='y' || input_value=='n')
    {
        break;
    }
    else
    {
        std::cout << "You have used " << current_attempt << " of " << MAX_USER_INPUT_ATTEMPTS << " attempts" << std::endl;
        ++current_attempt;
    }

    if( current_attempt > MAX_USER_INPUT_ATTEMPTS )
    {
        std::cout << "Warning: Maximum number of attempts reached." << std::endl;
        break;
    }
}

return 0;
}

Shaunta

Answer 7

Line-based input does not have to be verbose, you can make it succinct, with a single function you write once, that still handles corner cases:

bool yesno_repeat(char const* prompt) {
  using namespace std;
  while (true) {
    cout << prompt << " [yn] ";
    string line;
    if (!getline(cin, line)) {
      throw std::runtime_error("unexpected input error");
    }
    else if (line.size() == 1 and line.find_first_of("YyNn") != line.npos) {
      return line == "Y" || line == "y";
    }
  }
}

int main() try {
  if (yesno_repeat("Blow up?")) {
    take_off_every<Zig>(); // in the future, a zig is a nuclear missile...
  }
  return 0;
}
catch (std::exception& e) {
  std::cerr << e.what() << '\n';
  return 1;
}

Gayden

Answer 8

Here is a shorter way

char type;
while (type != 'y') 
{
    cout << "Were you admitted? [y/n]" << endl;
    cin >> type;   
}

Kettering

Answer 9

I've never coded in PERL/PHP before but based off of your question and the example here's a simple solution.

    char c;
while(true){
    cout << "Were you admitted? [y/n]" << endl;
    cin >> c;
    if(c == 'y')
        break;
}
cin.get(c);
return 0;

You will continue to be prompted until you enter a 'y' after entering a 'y' this simple program will exit.

Parlor

What does this solve?? Did you just decide to assume the example snippet is the entire main()? – Luthern

So, you just took Levi's code snippet (which was the question) removed the equality to 'n' condition (?!?!?!??!?!) and post it as a solution?!?!?!? – Pettway

Answer 10

The do...while construct is sort of made for this, but you can also do all the work in the loop condition:

while (std::cout << "Were you admitted [y/n]\n" && std::cin >> answer && !(answer == 'y' || answer == 'n'));

:)

And if you don't want to test the success of std::cin >> answer (e.g want to loop infinitely with Ctrl+Z), you can replace && for comma :)

Not entirely serious, even though methinks putting the prompt in the condition can sometimes enhance the logic of such loops (avoid break).

Reinert

Answer 11

I see two "problems" here. First one is the use of while( true ). I don't think that's a good practice (though probably this is a matter of taste for many people). Breaking inside a loop is however interesting for searching:

for(i = 0; i < MAX; ++i) {
  if ( v[ i ] == searchedElement ) {
      break;
  }
}

This is self-explanatory: you run until the end of the vector, though if the element is found, you break before reaching it. It is justifiable.

About getting information from the console, you can run into a lot of trouble reading directly from cin, for example, being returned the contents only until the first space if the input has one. getline() is an utility function that reads to a string, which is (nearly) always safe. getline() is defined in the utility header. You do also need the string header. And cstdio if you want to use EOF.

int main()
{
    int ch;
    std::string type;

    do {
        getline( std::cin, type );

        if ( cin.fail() ) {
            ch = EOF;
            break;
        }

        ch = tolower( type[ 0 ] );
    } while( ch != 'y' && ch != 'n' );

    // interesting things here...

    return 0;
}

Studding

Answer 12

Modifying what @McAden said, try this fix the bug where if you enter multiple characters, the test only checks the first letter.

    char type;
    char buffer[128];
    do
    {
      cout << "Were you admitted? [y/n]" << endl;
      cin >> buffer;
      type = buffer[0];
      cout << type << "\n";
    }while( !cin.fail() && type!='y' && type!='n' );

Manama

Answer 13

Here's another way to prompt for y/n. "While" you don't get what you want, "switch" on your input. Do whatever else you like with it; it just works.

#include "stdafx.h"
#include <iostream>

int main()
{
    bool play = true; // Initialize play to true.
    char choice;
    while (play) // While play is true, do the following:
    {
        std::cout << "Play again? [y/n] -> ";
        std::cin >> choice;
        switch (choice)
        {
        case 'n':           // We can fall through here because we
            play = false;   // don't do anything with 'y' anyway.
        case 'y':           // Remember; play is already true unless
            break;          // we changed it to false with 'n'.
        default:  // We'll simply assume anything else is a fail!
            std::cin.clear();
            std::cin.ignore(1024, '\n');
            break;
        }
        // Break out here and check the "while" condition again...
    }
    // When play becomes false, we exit the while statement.
}

This comes with no adverse after effects!

Tut

Answer 14

Another way to prompt for y/n . "Do" this "while" you don't get what you want. Do whatever else you like with it; it just works.

#include "stdafx.h"
#include <iostream>

int main()
{
    bool accepted;
    char answer;
    do
    { // Ask for 'y' or 'n' at least once
        std::cout << "Accepted? [y/n] -> ";
        std::cin >> answer;
        if (std::cin.fail())
        {                                // not a valid character?
            std::cin.clear();
            std::cin.ignore(1024, '\n');
        }
        if (answer == 'n')
        {                                // character is 'n'?
            accepted = false;
        }
        else
        {                                // character is 'y'?
            accepted = true;
        }
         //    not valid    |or|    not 'n'     |or|    not 'y'
    } while (std::cin.fail() || !(answer == 'n') || !(answer == 'y'));  
}

This also comes with no adverse after effects!

Tut

Answer 15

char yn;
bool choice;
do
    {
           
        cout<<"Would you like try again?\n";
        cout<<"Y for yes N for no: ";
        cin>>yn;
    
        while (yn != 'n'&& yn != 'N'&&yn != 'y'&&yn != 'Y')
        {
            cout<<"Y or N please: ";
            cin>>yn;
        }
        
        if (yn == 'n'||yn == 'N')
            choice = false;
        if(yn == 'y'||yn == 'Y')
            choice = true;
        
    } while (choice == true);

Fernanda