How can I remove the while(true) from my loop in Java?

Question

I've heard that using while(true) is a bad programming practice.

So, I've written the following code to get some numbers from a user (with default values). However, if the user happens to type in -1, then it will quit the program for them.

How should this be written then without a while(true)? I can think of a condition to make the while loop go off that will get caught right away without continuing on until the next iteration?

Here is how I have it now:

 public static void main(String[] args)
    {
        System.out.println("QuickSelect!");

        while (true)
        {
            System.out.println("Enter \"-1\" to quit.");

            int arraySize = 10;
            System.out.print("Enter the size of the array (10): ");
            String line = input.nextLine();
            if (line.matches("\\d+"))
            {
                arraySize = Integer.valueOf(line);
            }

            if (arraySize == -1) break;

            int k = 1;
            System.out.print("Enter the kth smallest element you desire (1): ");
            line = input.nextLine();
            if (line.matches("\\d+"))
            {
                k = Integer.valueOf(k);
            }

            if (k == -1) break;

            List<Integer> randomData = generateRandomData(arraySize, 1, 100);

            quickSelect(randomData, k);
        }
    }

Answer 1

while (true) is fine. Keep it.

If you had a more natural termination condition, I'd say to use it, but in this case, as the other answers prove, getting rid of while (true) makes the code harder to understand.

Answer 2

There is a Single Entry Single Exit (SESE) school of thought that suggests that you should not use break, continue or abuse exceptions to do the same for some value of abuse). I believe the idea here is not that you should use some auxiliary flag variable, but to clearly state the postcondition of the loop. This makes it tractable to formerly reason about the loop. Obviously use the stands-to-reason form of reasoning, so it is unpopular with the unwashed masses (such as myself).

public static void main(String[] args) {
    ...
    do {
        ...
        if (arraySize == -1)  {
            ...
            if (k != -1) {
                ...
            }
        }
    } while (arraySze == -1 || k == -1);
    ...
}

Real code would be more complex and you would naturally(!) separate out the inputing, outputting and core "business" logic, which would make it easier to see what is going on.

Answer 3

    bool exit = false;
while (!exit) {
    ...
    ...
    if (k == -1) {
        exit = true;            
    }
    else {         
        List <Integer> ....;
        quickselect(.......);
    }
}

But as has been said before, your while loop is a valid usage in this situation. The other options would simply build upon the if statements to check for the boolean and exit.

Answer 4

While having a loop like this is not technically wrong, some people will argue that it is not as readable as the following:

bool complete = false;

while (!complete)
{

    if (arraySize == -1)
    {
        complete = true;
        break;
    }
}

Additionally, it is sometimes a good idea to have a safety loop counter that checks to make sure the loop has not gone through, say, 100 million iterations, or some number much larger than you would expect for the loop body. This is a secure way of making sure bugs don't cause your program to 'hang'. Instead, you can give the user a friendly "We're sorry but you've discovered a bug.. program will now quit.." where you set 'complete' to true and you end the program or do additional error handling. I've seen this in production code, and may or may not be something you would use.

Answer 5

If you really don't like while(true) you can always go for for(;;). I prefer the latter because it seems less redundant.

Answer 6

while ( true ) is perfectly fine here, since the condition is really "while the user doesn't want to quit"!

Alternatively you could prompt for both the inputs on one line to simplify the logic, and use "q" for quit: this allows you to refactor the loop to "while ( !line.equals("q") )".

Answer 7

The problem is that you're doing an awful lot in that loop, rather than separating the functionality into simple methods.

If you want to stick to a procedural approach, you could move the reading of the array size and k into separate methods, and use the fact that the result of an assignment is the assigned value:

    for (int arraySize; ( arraySize = readArraySize ( input ) ) != -1;) {
        final int k = readKthSmallestElement ( input );

        List<Integer> randomData = generateRandomData(arraySize, 1, 100);

        quickSelect(randomData, k);
    }

However that's still a bit ugly, and not well encapsulated. So instead of having the two != -1 tests on separate variables, encapsulate arraySize, k and randomData in an object, and create a method which reads the data from the input, and returns either a QuickSelect object or null if the user quits:

    for ( QuickSelect select; ( select = readQuickSelect ( input ) ) != null; ) {
        select.generateRandomData();
        select.quickSelect();
    }        

You might even want to go to the next stage of creating a sequence of QuickSelect objects from the input, each of which encapsulate the data for one iteration:

    for ( QuickSelect select : new QuickSelectReader ( input ) ) {
        select.generateRandomData();
        select.quickSelect();
    }        

where QuickSelectReader implements Iterable and the iterator has the logic to create a QuickSelect object which encapsulates arraySize, k, the list and the quick select operation. But that ends up being quite a lot more code than the procedural variants.

I'd only do that if I wanted to reuse it somewhere else; it's not worth the effort just to make main() pretty.

Also note that "-1" doesn't match the regex "\\d+", so you really do have an infinite loop.