Program terminates instead of continuing integer input validation loop

Question

I'm asking the user to select a choice between items in a backpack by entering an integer corresponding to the item. But despite my current integer input validation code, the whole program terminates instead of redisplaying the choices and asking the user to enter a choice again. Is there anything problematic in the code below that may be causing this?

    int num;     
    do{
        std::cout << "Choose item to use." << std::endl;
        for(int i = 0; i < backpack->size(); i++){
           std::cout << i+1 << ". " << backpack->at(i) << std::endl;                        
        };
        std::cin >> num;
        if(!std::cin.fail()){
            if(num < 0 || num > (backpack->size())){
                std::cout << "Plese enter an integer in range." <<std::endl;
            }else{
                  break;
            };
        }else{
              std::cin.clear();
              std::cin.ignore(80, '\n');
              std::cout << "Invalid input. Please enter an integer." << std::endl;   
        };                      
    }while(std::cin.fail() || (num<0 || num > (backpack->size())));

It sounds like you may need to learn how to use a debugger to step through your code. With a good debugger, you can execute your program line by line and see where it is deviating from what you expect. This is an essential tool if you are going to do any programming. Further reading: [How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/) — NathanOliver, Jun 13 '17 at 13:33
Does the program *terminate* (`std::terminate`) or does just the loop end? — Daniel Jour, Jun 13 '17 at 13:44
Also please do not be afraid of whitespace. Your code is very hard to read because you don't seem to like spaces and blank likes — Tom Doodler, Jun 13 '17 at 13:44

score 2 · Answer 1 · answered Jun 13 '17 at 13:36

2

You have to assign an initial value to num. If you don't, it will contain a garbage value, which is possibly less than 0 or greater than backpack->size()-1, which will make the condition true.

int num = 0;

answered Jun 13 '17 at 13:36

Thomas James Tiam-Lee

691
3
9

There is no need to initialize `num`. It is assigned to by the stream extraction. – Buster Jun 13 '17 at 13:39
Due to the format of the loop num does not need to be initialized, in the do while loop, he will cin a value into num before it ever gets used. however if you wanna check for a fail on it it would be best to initialize to a value that isn't valid for his conditions so... `int num = -1;` – AresCaelum Jun 13 '17 at 13:42
Check `num` to see if the extraction failed? How would that work? You would need to check the stream (the OP is doing that more or less correctly). – Buster Jun 13 '17 at 13:44
@Buster yes, that is correct you need to check the stream to see if the cin failed. However, do you initialize your variables to a valid state, as successful, or to a valid index? The only time I do that is for a loop, otherwise I always initialize to a fail value, that way if there is a logic error in my code, it is easier to troubleshoot, since it will fail sooner rather then possibly working with bad conditions. – AresCaelum Jun 13 '17 at 13:51
I don't use rules of thumb, I do the right thing. Initializing a variable to a value that can never be used is pointless and usually gives a compiler warning. – Buster Jun 13 '17 at 13:55
@Buster difference in opinion, however, initializing a value to a fail state doesn't give a compiler warning, so initialize `int num = -1;` isn't a warning. it just initializes it to an index that is not valid for an array, if the code runs correctly, when num is done with the loop it wont be -1, instead it will be a valid index, if it is -1 there is an error in the code. You say it's pointless I say it helps tracking down bad logic. – AresCaelum Jun 13 '17 at 14:03
1

Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/146542/discussion-between-buster-and-eddge). – Buster Jun 13 '17 at 14:09

Nathan.Eilisha Shiraini · Answer 2 · 2017-06-13T14:03:49.273

1

Edit: Scraped last answer

The following clear the cin.fail() flag:

std::cin.clear();
std::cin.ignore(80,'\n');

So upon reaching the loop condition, cin.fail() returns false and the unitialized num most likely contains 0. Thus, your continuation condition does not pass and the loop returns.

As suggested by @Eddge, you should initialize num to an invalid value in regards to your condition, like -1.

edited Jun 13 '17 at 14:03

answered Jun 13 '17 at 13:38

Nathan.Eilisha Shiraini

628
1
8
24

I think he does want to continue the loop if num is negative or bigger then backpack size... But maybe not if cin is in a fail condition – AresCaelum Jun 13 '17 at 13:40
1

Indeed, re-reading the code helps ^^ Sorry! Gonna delete if OP confirms. – Nathan.Eilisha Shiraini Jun 13 '17 at 13:43
Currently, if I were to input something out of range in terms of item numbers, the validation works fine. But when I try inputting 'a' instead of, say, an integer between 1 to 5, the program terminates completely instead of asking the user to try inputting again. – Amos Wu Jun 13 '17 at 13:44
So yeah, I'm wanting the loop to continue as long as the input is not an item number between 1 and the backpack size-1. – Amos Wu Jun 13 '17 at 13:45
OK - gonna scrape answer, I think I have a new idea. – Nathan.Eilisha Shiraini Jun 13 '17 at 13:49
Actually, the upper range should be the backpack size itself. Got it mixed up with indexes. Will edit question accordingly. – Amos Wu Jun 13 '17 at 13:53
Re-wrote, according to what Thomas James Thiam-Lee and Eddge said – Nathan.Eilisha Shiraini Jun 13 '17 at 14:04
Just found that changing "std::cin.fail()" to "!(std::cin>>num)" in the while condition did the trick. Why do you think that is? By the way, thanks for all of your input! – Amos Wu Jun 13 '17 at 14:28
1

The only actually correct answer here. As another suggestion, the loop condition should not check what was already checked before, so `} while (true);` would be better. Even better is checking the actual input operation: `do { while (!(cin>>num)) { /* failure, continue, return if EOF */ } } while (! in_range(num));` – Daniel Jour Jun 13 '17 at 15:21

score 1 · Answer 3 · answered Jun 13 '17 at 14:05

After looking over the code a few times, I don't immediately see a problem with the logic, though I could be wrong. But from what I understand is the intention of the code, the condition in the while statement is redundant and unnecessary. You are already checking those same conditions in the if statements and breaking out of the loop when needed, so try using while(true) as your while statement and see if it fixes your problem.

score 0 · Answer 4 · answered Jun 13 '17 at 14:29

0

I just found that changing "std::cin.fail()" to "!(std::cin>>num)" in the while condition did the trick. Why do you think that is? By the way, thanks for all of your input!

answered Jun 13 '17 at 14:29

Amos Wu

37
5

Program terminates instead of continuing integer input validation loop

4 Answers4