0

I am working on a menu driven program to output an adjacency matrix for a homework assignment. When I put a character in as my input after I call any of the other 4 cases my loop runs infinitely and I can't figure out why.

This happens regardless to if I have case 5 active.

But if I enter a character as the first input then it closes properly as intended.

I stepped through the debugger and it seems that if a character is entered it takes that input to be 4 and never takes another input so it keeps printing the array over and over.

Can anyone explain what is wrong with this function? I only have the function here because the entire program is roughly 300 lines not counting comments. But through some testing I've narrowed my bug down to this specific function the others do what they are meant to. 

    void menu(char graph[][8])
{
    bool run = true;
    while (run == true)
    {
        int menuChoice;
        cout << "Welcome to the menu" << endl;
        cout << "Pick one of the following" << endl;
        cout << "1. add connection" << endl;
        cout << "2. delete connection " << endl;
        cout << "3. show total number of connections " << endl;
        cout << "4. show matrix " << endl;
        cout << " 5. to exit" << endl;
        cout << "Selection : ";
        cin >> menuChoice;
        switch (menuChoice)
        {
        case 1: addConnection(graph);
            break;
        case 2: deleteConnection(graph);
            break;
        case 3: showConnection(graph);
            break;
        case 4: showMatrix(graph);
            break;
        /*case 5:
            cout << "Exiting ...\n";
            run = false;
            break;*/
        default:
            cout << "Improper input " << endl; // for some reason this flies into infinite when a character is entered.
            cout << "Exiting ...\n";
            run = false;
            break;

        }
    }
}
Callat
  • 2,928
  • 5
  • 30
  • 47
  • Not able to reproduce with an MCVE: http://ideone.com/uX0Ez2 – kfsone Apr 20 '16 at 01:27
  • If I understand the question correctly, when you provide non-numeric input while performing a `cin >> menuChoice` (which is an `int`), you loop forever? – mah Apr 20 '16 at 01:28
  • Actually it works for me. So I guess the issue sits in the functions you called via cast 1...4 – DAG Apr 20 '16 at 01:32
  • @mah not quite if i plug in 1-4 and it runs the functions and comes back to the menu runs the break then comes again and asks for an input. On the second input if I put in a character the debugger reads it as a 4 if it's a non-number. And it loops forever. It stops taking inputs and keeps 4 and runs function in case 4 and repeats over and over. It's so weird to try to put it into a typed explanation. – Callat Apr 20 '16 at 02:00
  • @kfsone In your link main has the body of the function itself. But in my program the menu is a function that main calls.Would that make a significant difference? – Callat Apr 20 '16 at 02:28
  • @DAG I thought that but I tested several combinations of the functions and they worked fine. Which is why I'm convinced that it has to be the menu function because its the only thing that flies into infinite. – Callat Apr 20 '16 at 02:29
  • 1
    @Hikari You can do a simple test by removing all add/delete/show functions, namely just leave case 1...4 with breaks only. Then you can have a 'menu' only test. Would you mind to have a try? – DAG Apr 20 '16 at 04:00
  • @DAG The same thing happened. Sam's answer below was a good explanation. It doesn't neccessarily hold `4` but it holds the last option selected. When a character is input the `>>` operator breaks and it's fail bit is set active. Because it's failed and virtually ignored by my program it can't take another input or escape the loop. It doesn't do this if the character is the first choice because the value of the input is uninitialized and triggers my default case. I fixed it by adding an `if(cin.fail()) cout << "Char input fail" << endl; cin.clear(); break;` so my issue is resolved. – Callat Apr 20 '16 at 04:10

1 Answers1

5

This is a textbook case why operator>> should never handle interactive line-oriented input.

If your intent is to enter a line of text, then that's what std::getline() is for.

This code uses operator>> on an int.

if a character is entered it takes that input to be 4

No, it doesn't. The operator>> conversion fails. The menuChoice is uninitialized, contains random junk. It may be '4', it may be 'X', it may be some unprintable character. The sky's the limit.

But more important, std::cin has now has the failed bit set, and any further attempts to read from std::cin will fail, until the stream status is clear()ed. The code never does it, and just keeps looping over and over again, vainly attempting to operator>> the next character, which, of course, does not do anything since the stream is in the failed state. And since the code doesn't check the status of the input stream, it has no idea what's going on.

Do not use operator>> to read std::cin. If you want to read a line of text, use std::getline(), and parse whatever you read using an independent std::istringstream.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • I considered that but I don't want to read text I only want integers but if my grader puts in a character to test it after running a function it breaks. If I use `getline(cin,input);` how would that change menuChoice to being initialized when menuChoice is type `int`? – Callat Apr 20 '16 at 02:03
  • It wouldn't. It would be still your responsibility to correctly handle invalid input. But at least you won't have to worry about `std::cin`'s state getting messed up. – Sam Varshavchik Apr 20 '16 at 02:19
  • Well how would you purpose that I handle the case of the invalid input of char? The first thing that comes to mind is an assert or try/catch but I don't know how to test for a data type itself. What do you suggest? – Callat Apr 20 '16 at 02:25
  • 1
    try/catch won't do anything. By default, a conversion failure does not throw an exception. You didn't get a thrown exception with your current code, when a conversion failure happened; and I have no idea why you expect one to be thrown tihs time.. You just take the line that was read into a `std::string`, construct a `std::istringstream` from it, execute `operator>>` with it, then check its `fail()` to determine whether the conversion failed. – Sam Varshavchik Apr 20 '16 at 02:28
  • I'm VERY unfamiliar with string stream I only used it once to create a map for my last project. But I will look into that. Do you have an alternative that you can suggest by any chance? – Callat Apr 20 '16 at 02:31
  • 1
    No other alternative. This is the right way to do it, and in most cases, there's only one right way to do it. You should get used to always encountering various bits and pieces of C++ you are unfamiliar with it, and need to learn in order to do something the right way. After >20 years of working with C++, I still learn new things every once in a while, that I did not know before. If you expect to have a career as a C++ developer, the ability to learn new stuff all the time is a part of the job. – Sam Varshavchik Apr 20 '16 at 11:04
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/109709/discussion-between-hikari-and-sam-varshavchik). – Callat Apr 20 '16 at 17:05