C++ do/while loop with two different conditions

Question

I want the do while loop to check to see if the entered input is R OR P. I think it is checking for both and when I get to that part when I run, it pauses for a minute and then I get "CPU Limit Exceeded (core dumped). On another related note, am I in danger of breaking something?

/************************************************/
/* Name: servcode                               */
/* Description: Get service type                */
/* Parameters: N/A                              */
/* Return Value: servcode                       */
/************************************************/

char servcode()
{
  char servcode = 'a';   // Define variable for service code
  char serviceyn = 'n';  // Define variable for user verify
  int i = 1;             // Define variable for sentinel loop

  do {
    cout << "\n" << "\n" << "Please enter your service code, [R]egular or [P]remium: " << "\n";
    cin >> servcode;
    while ((servcode != 'R', 'P') && (i < 3));
    {
        cout << "\n" << "Error - invalid service code, please try again.";
        cout << "\n" << "Please enter your service code: ";
        cin >> servcode;
        i++;
        if (i == 3)
        {
            cout << "\n" << "Too many invalid attempts, program terminating." << "\n"
             << "Have a nice day. " << "\n" << "\n";
            exit (0);
        } //end if
    } //end while
    cout << "\n" << "You entered: " << servcode << "\n"
     << "Is that correct? [y,n]";
    cin >> serviceyn;
  } while (serviceyn != 'y'); // end do/while loop

  return servcode;
}

A canonical question is *[Multiple conditions in a C 'for' loop](https://stackoverflow.com/questions/16859029/)*. But there must be one from 2008 or 2009. — Peter Mortensen, Sep 01 '22 at 22:54

NPE · Accepted Answer · 2011-10-19T15:35:09.457

7

The correct syntax is:

while (servcode != 'R' && servcode != 'P' && i < 3)

Note the expanded comparison and the removal of the semicolon at the end:

(servcode != 'R', 'P') is valid C++ but doesn't do what you're expecting it to do;
the semicolon makes the statement into a loop with an empty body, so it continues executing forever since the loop condition never changes.

edited Oct 19 '11 at 15:35

answered Oct 19 '11 at 15:27

NPE

486,780
108
951
1,012

so thats why the server stopped my program, i created a never ending/never changing loop that was using up resources? – Sam LaManna Oct 19 '11 at 15:30
is that a basic buffer overflow attack or am i thinking of somehting else? – Sam LaManna Oct 19 '11 at 15:33
@SamLaManna: No, it's not a buffer overflow. The extraneous semicolon makes the loop into an infinite one, since nether `servcode` nor `i` ever change. – NPE Oct 19 '11 at 15:34
ok thanks, just making sure im not in danger of breaking somehting – Sam LaManna Oct 19 '11 at 15:38

score 4 · Answer 2 · answered Oct 19 '11 at 15:27

4

Change:

while ((servcode != 'R', 'P') && (i < 3));

to:

while ((servcode != 'R') && (servcode != 'P') && (i < 3))

Note the removal of an unwanted semicolon.

answered Oct 19 '11 at 15:27

Paul R

208,748
37
389
560

SE Does Not Like Dissent · Answer 3 · 2011-10-19T15:35:06.653

0

(servcode != 'R', 'P')

Should be:

(servcode != 'R') && (servcode != 'P')

Kudos to PaulR for correction. My brain is not in gear.

edited Oct 19 '11 at 15:35

answered Oct 19 '11 at 15:28

SE Does Not Like Dissent

1,767
3
16
36

No - it needs to be `&&`, not `||`. – Paul R Oct 19 '11 at 15:30
PaulR: Correction, yes. Ampersands required. Not ors. – SE Does Not Like Dissent Oct 19 '11 at 15:32
@PaulR: You were correct. Loop logic is indeed inverse to if-statement logic ( if(servcode == 'R' || servcode == 'P') versus (servcode != 'R' && servcode != 'P') ). – SE Does Not Like Dissent Oct 19 '11 at 15:36

score 0 · Answer 4 · answered Oct 19 '11 at 15:30

You need to do something like this:

while(something != 'a' && something != 'b')

You're using the comma operator, which discards the results of every expression except the last, so this:

while(something != 'a', 'b')

Will compare something with a, ignore the result, and use 'b' as the condition of the loop. 'b' is a non-zero value, so it's always true and the loop goes on forever (or until memory runs out, or something else stops it).

score 0 · Answer 5 · answered Oct 19 '11 at 15:35

Let's break down this statement and see what it's doing. I've put in some extra punctuation that does not change the meaning of the statement.

while (((servcode != 'R'), ('P')) && (i < 3)) { };

The comma operator separates two different expressions and returns the value of the second one. The first part probably does what you expect, but the second part is just a literal character 'P' which is always true!

The semicolon at the end of the statement marks the body of what will be executed as the while loop. It's a do-nothing which does not change the value of servcode or i, so obviously once you enter the loop you will never leave it. This is a common mistake.

C++ do/while loop with two different conditions

5 Answers5

Linked