-2

I am trying to write a code to prompt user input from cin.

int main()
{
    int year;
    cout << "Enter a valid year: ";
    cin >> year;

    while (cin.fail())
    {
        cout << "Re-enter a valid value! ";
    }
}

However, the compiler keeps printing the line "Re-enter a valid value! " unterminatedly when I enter a non-integer type. I don't know what's wrong about my code! Can somebody please correct it? Thank you very much.

bffaf01
  • 27
  • 1
  • 1
  • 1
  • So many questions... why does "the compiler print" anything? That's not what compilers do. And why "enter a *valid* year"? Why not just "enter a year"? When would you ever need to specify that you want something *valid*? – Kerrek SB Jan 05 '17 at 01:27
  • @KerrekSB: I'm sorry for my lack of understanding of the terminologies, but can you specify what needs to be changed in my code? – bffaf01 Jan 05 '17 at 01:30
  • This looks like a duplicate: http://stackoverflow.com/questions/5655142/how-to-check-if-input-is-numeric-in-c/5655685#5655685 – ebyrob Jan 05 '17 at 01:33

3 Answers3

5

Try this:

bool done = false;
int year;

for (std::string line;
     std::cout << "Enter a year: " && std::getline(std::cin, line); )
{
    std::istringstream iss(line);
    if (iss >> year >> std::ws && iss.get() == EOF) { done = true; break; }
    std::cerr << "Failed to parse input '" << line << "', please try again.\n";
}

if (!done) { std::cerr << "Premature end of input.\n"; }
else       { std::cout << "Input: " << year << "\n";   }
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • 3
    You make my day. Finally someone do proper input handling on cin ;). And I am also sad for bffaf01 as probably he will not understand a thing from your code. – Logman Jan 05 '17 at 01:44
  • @Logman what's wrong with the more typical `istream::clear()` and `istream::ignore()` approach without an extra stream? – ebyrob Jan 05 '17 at 01:52
  • 1
    I know it's a command-line not a database or website, but isn't: `'" << line << "'` un-sanitized I/O? Maybe I've been writing web apps and worrying about cross site scripting too long? – ebyrob Jan 05 '17 at 02:06
  • @ebyrob I get this answer as a little joke. Answer is correct in every aspect, it's really well crafted but it's way too complicated for someone who do no know why `while (cin.fail())` is not working. – Logman Jan 05 '17 at 02:10
  • @ebyrob it's not like we're doing "select * from " << line – M.M Jan 05 '17 at 02:12
  • @ebyrob: I'm not sure what you mean by "sanitized I/O". Perhaps if you give me a definition I can answer your question. – Kerrek SB Jan 05 '17 at 02:19
  • @KerrekSB secure = sanitized in this case. https://en.wikipedia.org/wiki/Secure_input_and_output_handling I know there are shell vulnerabilities in various operating systems with regard to say creating filenames, I can't recall if there are any with simply reflecting input to output at the command line. – ebyrob Jan 05 '17 at 02:27
  • @Logman: It's not really a joke. The code parses line-based input and performs pragmatic validity checking and emits somewhat useful diagnostics on error. You could vary some details, and it would help if the problem were stated more precisely, but without further information I'd consider this a good starting point. – Kerrek SB Jan 05 '17 at 02:33
  • @KerrekSB I never said that something is wrong with your code. As I said it's really well crafted and one can use it as a template for good input handling. But not long time ago I teach someone c++ and I am 90% sure that bffaf01 do not understand a single line of your code. That why I assumed you make a little joke making this code. – Logman Jan 05 '17 at 02:53
4

This is my approach :

int main() {
    int year;

    while (true) {
        cout << "Enter a valid year: ";
        cin >> year;
        if (cin.fail()) {
            cin.clear(); cin.ignore();
            cout << "Re-enter a valid value! " << endl;
        } else break;
    }
    return 0;
}
  • I seem to recall something about `if(cin)` or `if(!cin)` somehow being superior to `ios::good()`, `ios::bad()`, and `ios::fail()` but it could be `cin.fail() == !cin` in all cases and I just forget that all the time. – ebyrob Jan 05 '17 at 02:15
  • Just as a note, this code [accepts `xyz1234abc` as valid input](https://ideone.com/6xUCrC). I don't know whether that behaviour is preferable over the alternative, but it's worth calling out the difference. – Kerrek SB Jan 05 '17 at 02:36
0

The problem you've described is here:

while (cin.fail())
{
    cout << "Re-enter a valid value! ";
}

It looks like you don't fully understand what's happening here yet, so I will break this down for you.

The code translates to something like this:

"While cin.fail() is true, output "Re-enter a valid value! " to the console"

Which it will do continuously because there is no way to break out of the loop.

I don't think cin.fail() is what you want to use here, it looks like you want an algorithm to tell you if the date is valid or not and, if it's not valid, to repeat the query to the user so they can enter good data. Conceptually this is good practice, you're on the right track, but you need to learn a bit more.

I would suggest reading some tutorials on input/output and also looking into input validation.

This is an excellent resource for learning C++ http://www.cplusplus.com/doc/tutorial/

And here is some info on input validation http://www.cplusplus.com/forum/beginner/121194/

Atrixium
  • 51
  • 3
  • Input validation is only half the story. If you attempt formatted input directly from `cin`, you also need to reset the error state on the stream object. – Kerrek SB Jan 05 '17 at 14:34