C++ Question C++ Newb stuck in a loop

[TR1978]

Hi,

I've started trying to learn C++, in the hopes of maybe doing something with Orbiter in the future.

With the basic stuff I have learned, I decided to try writing a decimal-binary converter, but I'm already stuck at the beginning.

The following code gets stuck in an infinite loop if i select anything else than 0, 1, 2, 3, 4, 5, 6, 7, 8 or 9. Can anyone tell me why?

#include <iostream>

using namespace std;

int main()
{
    for( int b = 0; b == 0;)
    {
        int iSelection = 0;
        cout << "*************************************" << endl
             << "*            ---Menu---             *" << endl
             << "*************************************" << endl
             << "1 - Convert from decimal to binary   " << endl
             << "2 - Convert from binary to decimal   " << endl
             << "3 - Exit program                     " << endl << endl
             << "Please choose: ";

        cin >> iSelection;

        if( iSelection == 1 )
            cout << "Chose 1n";
        else if( iSelection == 2 )
            cout << "Chose 2n";
        else if( iSelection == 3 )
            b = 1;
        else if( iSelection == 0 )
            cout << "nWrong selection" << endl;
        else
            cout << "nWrong selection" << endl;

    }

    return 0;
}

 

[Artlav]

Please clarify "The following code gets stuck in an infinite loop", since there is already an infinite loop in the code.

 

[Zatnikitelman]

Your for loop is kinda messed up. you've got b initialized to 0, but the terminating condition is if b is equal to 0, you also need an update condition after the last semi-colon. This tells the loop what to do at the end of the for loop. For many things, it's simply b++.

 

[Artlav]

Your for loop is kinda messed up. you've got b initialized to 0, but the terminating condition is if b is equal to 0, you also need an update condition after the last semi-colon. This tells the loop what to do at the end of the for loop. For many things, it's simply b++.

It looks like this for loop is not for looping but is just a crude replacement for a goto - if the selection is wrong, loop and try again.

 

[TR1978]

Please clarify "The following code gets stuck in an infinite loop", since there is already an infinite loop in the code.

Hehe, thats my problem, I don't see why its looping infinitely.

Your for loop is kinda messed up. you've got b initialized to 0, but the terminating condition is if b is equal to 0, you also need an update condition after the last semi-colon. This tells the loop what to do at the end of the for loop. For many things, it's simply b++.

Do I really need the update condition? My understanding of the for loop is:

(int b = 0; //Initialize b to 0
b == 0 // keep looping as long as b == 0

Then when I set b to 1 in the loop, it exits. Is this wrong?

It looks like this for loop is not for looping but is just a crude replacement for a goto - if the selection is wrong, loop and try again.

So a goto line would be better here? I haven't read about that yet, so I thought I'd try to make it without it. Guess ill go google "c++ goto"

 

[Artlav]

I don't see why its looping infinitely.

Well, why isn't the question, what you expect is.
I didn't exactly understood where you encounter the infinite loop.
Does it not exit when you press 3?
Does it lock up if you type in 42?
Does it do something else?
Etc...

My understanding of the for loop is:
...
So a goto line would be better here?

Your understanding is correct, and it does exactly what a goto will be doing, only implicitly.

 

[TR1978]

I made it work now using goto, thanks for that tip.

Well, why isn't the question, what you expect is.
I didn't exactly understood where you encounter the infinite loop.
Does it not exit when you press 3?
Does it lock up if you type in 42?
Does it do something else?
Etc...

Sorry, I'll try to explain better:

1 and 2 worked as expected (triggered the correspondig if statements. 3 exited the program. 4 to 9 and 0 made it print "Wrong selection".. All as I intended.

But, if I for instance typed in a "g", the menu just got printed over and over, without stopping, just rushing upwards forever. I don't understand why it just passed by the cin statement at that point.

 

[dbeachy1]

In most cases, though, in the interests of code readability it's best to avoid 'goto' statements where possible ('goto' is often useful for error handling, however, when you need to jump to a common error handler from inside nested loops). Granted, this could be a topic of debate.

In any case, the next-to-last condition in the 'if...else' block is redundant since both blocks do the same thing:

else if( iSelection == 0 )  // this is redundant
    cout << "\nWrong selection" << endl;  // this is redundant
else
    cout << "\nWrong selection" << endl;

There is no need to have redundant handlers; the final 'else' condition is sufficient:

else
    cout << "\nWrong selection" << endl;

One more thing: while the 'b == 0' loop works for infinite loops like that, it is often more elegant (and more readable) to code it like this:

    for (;;)  // loop until break
    {
        int iSelection;  // no need to init this to 0 since we reassign it below anyway
        cout << "*************************************" << endl
             << "*            ---Menu---             *" << endl
             << "*************************************" << endl
             << "1 - Convert from decimal to binary   " << endl
             << "2 - Convert from binary to decimal   " << endl
             << "3 - Exit program                     " << endl << endl
             << "Please choose: ";
 
        cin >> iSelection;
        if (cin.fail())    // in case user entered non-numeric characters
        {
            cout << "\nInvalid input." << endl;
            cin.clear();   // clear the error
            char junk;
            cin >> junk;  // flush the invalid character
         }
        else if( iSelection == 1 )
            cout << "Chose 1\n";
        else if( iSelection == 2 )
            cout << "Chose 2\n";
        else if( iSelection == 3 )
            break;   // exit loop and terminate
        else
            cout << "\nWrong selection" << endl;
     }

But, if I for instance typed in a "g", the menu just got printed over and over, without stopping, just rushing upwards forever. I don't understand why it just passed by the cin statement at that point.

That's because 'g' (or other non-numeric characters) cannot be converted to an integer, and so the 'cin' call fails. When that happens the character is not removed from the input buffer, and so when 'cin' is encountered again in the next loop the 'g' is processed (and fails) again. The solution is to check for an error using 'cin.fail' as follows:

cin >> iSelection;
if (cin.fail())    // in case user entered non-numeric characters
{
    cout << "nInvalid input." << endl;
    cin.clear();   // clear the error
    char junk;
    cin >> junk;  // flush the invalid character
}
else if( iSelection == 1 )  // ....continues....

 

[TR1978]

Thanks for that detailed explanation dbeachy1. Helps alot.

 

[Rathelm]

Hehe, thats my problem, I don't see why its looping infinitely.



Do I really need the update condition? My understanding of the for loop is:

(int b = 0; //Initialize b to 0
b == 0 // keep looping as long as b == 0

Then when I set b to 1 in the loop, it exits. Is this wrong?



So a goto line would be better here? I haven't read about that yet, so I thought I'd try to make it without it. Guess ill go google "c++ goto"

I would prefer to use a while loop instead.

while (!bQuit)
{
    // Do your code here and set bQuit = TRUE when
    // you want to exit the program.
}
 
return 0;