Whats wrong with this while loop?

Question

    boolean r = false ; int s = 0 ;
    while (r == false) ; 
    {
        s = getInt() ; 
        if (!(s>=0 && s<=2)) System.out.println ("try again not a valid response") ; 
        else r = true ; 
    }

The text never displays itself even when a 3 or a 123 is entered and the loop never terminates. Whats wrong here?

Answer 1

You have a semicolon after the condition. When you use braces to specify a block for your while you don't use a semicolon.

Answer 2

Remove the ';' after while.

Answer 3

Others have pointed out the bug, but your code is scary in other ways that will eventually trip you up:

if (!(s>=0 && s<=2)) System.out.println ("try again not a valid response") ; 
else r = true ; 

That's bad because you can easily intend more than one statement to run in the case of the if or else clause. Use curly braces and avoid placing conditional statements on a single line:

if (!(s>=0 && s<=2))
{
    System.out.println ("try again not a valid response");
}
else
{
    r = true;
}

It's easier to read and far less likely to introduce hard-to-see bugs.

Answer 4

while(r == false)

should be

while(!r)

Despite what everyone else said about the semicolon, that is what I think is wrong with it :)

Answer 5

+1 to Daniel DiPaolo. I thought I'd post a separate answer to provide clarification of why this is the case.

While loops in Java can be written in one of two ways. If there is just one line to the body of the loop, you can write them in a short-hand fashion:

while (true)
    System.out.println("While loop");

This will print out "While loop" on the console until the program ends. The other option is to specify a loop body between braces, as you have done above:

int i = 0;
while (i < 10) {
    System.out.println("i = " + i);
    i++;
}

This will print out "i = 0", "i = 1", ..., "i = 9" each on a separate line.

What the code you posted does is confuse the two. In the short-hand while loop, the Java parser expects to find a statement between the while loop condition and the semi-colon. Because it does not find a statement here, the while loop runs, but does nothing; it has no body. Furthermore, because the loop has no body, there is no opportunity for your variable r to assume a new value; the condition always evaluates to true and the loop never exits.

If you were to negate the condition in the while loop in your example, i.e.,

boolean r = false ; int s = 0 ;
while (r != false) ; 
{
    s = getInt() ; 
    if (!(s>=0 && s<=2)) System.out.println ("try again not a valid response") ; 
    else r = true ; 
}

(note I left the erroneous semicolon in there), you would find that your intended loop body would execute precisely once, as the loop would never run.

Answer 6

In addition to other comments, you should also change the if to

if (s < 0 || s > 2)

It's much more understandable this way.

Answer 7

Unrelated answer, I really really recommend you to follow Sun's style guidelines.

boolean r = false ; 
int s = 0 ;
while (r == false) {
    s = getInt() ; 
    if (!(s>=0 && s<=2)) {
        System.out.println ("try again not a valid response") ; 
    } else {
      r = true ;
    } 
}

You could get rid of the r variable and the if/else condition if you evaluate the result in the loop it self.

int s = 0;

while( ( s = getInt() ) < 0 || s > 2 ) {
    System.out.println( "Try again, not a valid response");
}