CODEWITHSUNDEEP
c++while-loopswitch-statementinfinite-loop
Joe Sanders
Joe Sanders

Reputation: 41

Methods in which to fix poor while-switch loop?

My switch case is supposed to be within the while loop, but it is having complications. I want the loop to end when the user inputs 1,2, or 3, while continuing to loop if any other value. 

#include <iostream>
using namespace std;

int main() {
int k = 0;
enum CompanyRole {EXECUTIVE = 1, MANAGER = 2, STAFF = 3};

cin >> k; 
while((k != 1) || (k!=2) || (k!=3)){
            switch(k)
            {
                    case EXECUTIVE:
                    cout << "Executive\n";
                    break;
                    case MANAGER:
                    cout << "Manager\n";
                    break;
                    case STAFF:
                    cout << "Staff\n";
                    break;
                    default:
                    cout << "Wrong role. Enter again.\n";
                    cin >> k;
                    break;

            }
    std::cin >> k;
    }

return 0;
 }

Upvotes: 2

Views: 42

Answers (3)

user2736738
user2736738

Reputation: 30926

Here whenever you have suppose value for k is 1 there is one conditional statement which evaluates to true and enters the loop. (This will also hold for k=2 or k=3).

It will be while((k != 1) && (k!=2) && (k!=3)). Think this way, earlier you were saying - you will continue to the loop if either of them is true. Because in if either of the condition is true - the whole expression evaluates to true - which basically make the loop iteration.

Here by putting those && we are ensuring that it is not one of 1 ,2 or 3. That is what you wanted to achieve.

The condition satisfying which the control enters the loop is those very conditions which are taken into account in switch. Solution:- Redesign the code - more explicitly, put the switch cases after the while statetment. Make sure you understand what each component does and redesign.

Upvotes: 2

PaulMcKenzie
PaulMcKenzie

Reputation: 35454

Since the loop is performed at least once, you should use a do-while instead of a while loop. A do-while loop is always performed at least once, so the test to see whether you exit the loop is done at the bottom, not at the top.

In addition to that, you could simply introduce a bool variable and set it to false if the input is "bad". This makes the code much easier to vision and to follow.

Example:

#include <iostream>
using namespace std;

int main()
{
    int k = 0;
    enum CompanyRole { EXECUTIVE = 1, MANAGER = 2, STAFF = 3 };
    bool ok;
    do
    {
        ok = true;  // assume input is good
        cin >> k;   // get the input
        switch (k)
        {
            case EXECUTIVE:
                cout << "Executive\n";
            break;
            case MANAGER:
                cout << "Manager\n";
            break;
            case STAFF:
                cout << "Staff\n";
            break;
            default:
                ok = false;  // input is bad.
                cout << "Wrong role. Enter again.\n";
            break;
        }
    } while (!ok);  // if input is bad, loop again
}

At the top of the loop, we assume the input is good (ok = true;), and if it's bad, set ok to false. Then the test is for ok being false.

The above is much easier to follow, no having to deal with getting logical and and or erroneously stated, basically not too much thinking is involved. If the input is "bad", just indicate it's bad and test for it.

Upvotes: 0

Stephen Newell
Stephen Newell

Reputation: 7863

I'd rewrite your code to look like this:

while((k != 1) && (k!=2) && (k!=3)){
    std::cout << "Wrong role. Enter again.\n";
    std::cin >> k;
}
switch(k) {
    // same as before
}

Your while is acting as a trap for valid input, so you don't need to test for good input in there. Let the while naturally exit once you have a valid value for k, then operate on k. It also avoids a few other small issues in your code.

Upvotes: 0

Related Questions