Help with while loop

I cant figure out how to enter anything else if i screw up. Im supposed to be able to enter a choice and if it is right then terminate the loop and continue, but if i enter a string or something then im supposed to be able to re enter a choice but i cant figure out how to do that.

while(select_loop != false)
    {
        cin >> select;

        if(select == 1 || select == 2)
        {
            select_loop = false;
        }
        else
        {
            cout << "That is not a valid choice" << endl;
        }
    }

Last edited on

http://www.cplusplus.com/forum/beginner/99203/#msg534078

Hi Ch1156,

I much prefer to do these things with a switch:

Btw, line 1 can be written like this:

while(select_loop)

or the inverse:

while(!select_loop)

Hope all goes well :-)

billywilliam (79)

For most programs I write, I keep the loop infinite and explicitly continue/break at the end of every incoming message and its assigned functions.

void main(){
       while(){
              cmessage _userinput=cmessage::GetNewMsg();
              ParseInput(_userinput);
              continue;
       }
       return;
}

void ParseInput(cmessage _userinput){
       switch(_userinput){
       case "1": DoStuff();
              return;
       case "2": DoDifferentStuff();
              return;
       default:
              cout << "That is not a valid choice" << endl;
              return;
       }
       //should never get here, but I'll add a return; because I'm an imbecile.
       return;
}

I wouldn't consider myself adept in C++ just yet, and there may very well be a more efficient way to handle this.

Ok but that still doesnt answer my question, if i enter the wrong thing then the program quits on me how can i keep it from doing that? here is my code

    cin >> select;

    switch(select)
    {
        case 1:
            break;
        case 2:
            break;
        default:
            cout << "Not a valid input" << endl;
    }

billywilliam wrote:
I wouldn't consider myself adept in C++ just yet, and there may very well be a more efficient way to handle this.

Well look at the link I posted.

There are several problems with your code (if you don't mind me pointing them out)

It is : int main() not void main() always and for ever. Are you using a really ancient compiler like turbo C++ 3.0 which allows this despite all the C / C++ standards?

Although there are many valid reasons for using infinite loops, I am wary of beginners who use them routinely, mainly out of laziness. Your code is a classic misuse: it doesn't have a break statement - so it is a true infinite loop.

I can understand why people use a return statement to return early from a void function - but I have never understood why people put one at the end.

It is more usual to have a break statement at the end of each case in a switch - you are using return instead. It is not wrong, but could cause problems later when you modify the code.

Ch1156 wrote:
Ok but that still doesnt answer my question, if i enter the wrong thing then the program quits on me how can i keep it from doing that?

If you look at my code you will see that the switch is inside a bool controlled while loop. It also has a quit option in the menu with a corresponding case in the switch.

Last edited on

I still cant get it working

So what does your code look like now?

Should be pretty easy to figure out - my code shows everything you need to know.

while( std::cout << "Please make a selection -> " << std::endl && std::cin >> select && ( select != 1 || select != 1 ) )
{
    std::cin.clear();
    std::cin.ignore( std::numeric_limits<std::streamsize>::max() , '\n' );
    std::cout << "That is not a valid selection." << std::endl;
}

*forgot a closing " on line 1...

Last edited on

@giblit

While that code may look more elegant, it is not scalable because of this part:

(select != 1 || select != 1)

what if there are 20 options? And should it be && and not || ?

Those sort of messy statements are terrible IMO.

I meant 1 then 2 and yeah sorry I meant && not ||.
He only had two options thats why i said that..
He could of also done something like this if this is perferred..

bool temp( false );
while( std::cout << "Please make a selection -> " << std::endl && std::cin >> select && !temp )
{
    switch( select )
    {
    case 1: case 2:
        temp = true;
        break;
    default:
        std::cin.clear();
        std::cin.ingore( std::numeric_limits<std::streamsize>::max() , '\n' );
        std::cout << "That is not a valid selection." << std::endl;
        break;
    }
}

Not sure if this is any better though but a little neater I suppose.

Edit or even this

bool temp( false );
while( !temp )
{
    std::cout << "Please make a selection -> " << std::flush;
    std::cin >> select;
    switch( select )
    {
     case 1: case 2:
         temp = true;
         break;
    default:
         std::cin.clear();
         std::cin.ignore( std::numeric_limits<std::streamsize>::max() , '\n' );
         std::cout << "Invalid selection." << std::endl;
    }
}

Last edited on

JLBorges (13770)

> it is not scalable because of this part:

Only if you don't know how to factor code.

> what if there are 20 options?

What if one writes a function to validate?

while( .... && valid(selection) ) would be a lot less messy or terrible than refusing to write a function and consequently putting those 20 checks inside the while loop.

> Not sure if this is any better though

No. It is a lot worse.

Last edited on

http://www.stroustrup.com/Software-for-infrastructure.pdf

JLBorges wrote:
`while( .... && valid(selection) )` would be a lot less messy or terrible than refusing to write a function and consequently putting those 20 checks inside the while loop.

Now I know you have vastly more knowledge & experience than me, but I cannot help arguing this (or at least enquiring why you think your solution is better). I am always ready to learn something new, but I don't understand why you are saying this at the moment.

The checks inside the while loop are the switch statement with a default case. What would your valid function look like?
Which is worse:

1. a relatively complicated while condition with a function call while( std::cout << "Please make a selection -> " << std::endl && valid(selection) )

2. very simple conditions while(!Quit) and switch(input)

IMO it is simple, scalable & straight forward.

giblit's last code snippet is almost the same as what I had in the first place.

I was reading this yesterday:

Stroustrup talks about keeping things simple in this section:

Stroustrup wrote:
Prefer highly structured code It isn’t enough to be disciplined in our specification of data structures and interfaces: we must also simplify our code logic. Complicated control structures are as danger- ous to efficiency and correctness as are complicated data structures.

I don't mean to be trite - but isn't my code a good example of that?

So keeping it simple is to put the std::cout and std::cin one line farther down versus inside the while loop? I don't see the logic.

let me rephrase this..
putting the std::cout and std::cin inside of the while loop instead of inside of the while loop declaration

while( std::cout << "Something" << std::endl && std::cin >> variable )
{
}
//versus
while()
{ 
    std::cout << "Something" << std::endl;
    std::cin >> variable;
}

Last edited on

TheIdeasMan wrote:
2. very simple conditions `while(!Quit)` and `switch(input)`

I don't why this seems so difficult.

Ok well im close to figuring it out, i have this

while(select != 1 && select != 2)
    {
        cin.clear();
        cin.ignore(50, '\n');
        cin >> select;
    }

But i have to enter the number twice before it does anything, why?

Try putting the cin >> select before the clear and ignore. By the way that will not error check for letters and stuff they way you have it.

well that doesnt work either, what do i do? this is the only thing i need to complete my program.