Are "while(true)" loops so bad?

JHarnach picture JHarnach · Jul 27, 2011 · Viewed 178k times · Source

I've been programming in Java for several years now, but I just recently returned to school to get a formal degree. I was quite surprised to learn that, on my last assignment, I lost points for using a loop like the one below.

do{
     //get some input.
     //if the input meets my conditions, break;
     //Otherwise ask again.
} while(true)

Now for my test I'm just scanning for some console input, but I was told that this kind of loop is discouraged because using break is akin to goto, we just don't do it.

I understand fully the pitfalls of goto and its Java cousin break:label, and I have the good sense not to use them. I also realize that a more complete program would provide some other means of escape, say for instance to just end the program, but that wasn't a reason my professor cited, so...

What's wrong with do-while(true)?

Answer

Jon Skeet picture Jon Skeet · Jul 27, 2011

I wouldn't say it's bad - but equally I would normally at least look for an alternative.

In situations where it's the first thing I write, I almost always at least try to refactor it into something clearer. Sometimes it can't be helped (or the alternative is to have a bool variable which does nothing meaningful except indicate the end of the loop, less clearly than a break statement) but it's worth at least trying.

As an example of where it's clearer to use break than a flag, consider:

while (true)
{
    doStuffNeededAtStartOfLoop();
    int input = getSomeInput();
    if (testCondition(input))
    {
        break;
    }
    actOnInput(input);
}

Now let's force it to use a flag:

boolean running = true;
while (running)
{
    doStuffNeededAtStartOfLoop();
    int input = getSomeInput();
    if (testCondition(input))
    {
        running = false;
    }
    else
    {
        actOnInput(input);
    }
}

I view the latter as more complicated to read: it's got an extra else block, the actOnInput is more indented, and if you're trying to work out what happens when testCondition returns true, you need to look carefully through the rest of the block to check that there isn't something after the else block which would occur whether running has been set to false or not.

The break statement communicates the intent more clearly, and lets the rest of the block get on with what it needs to do without worrying about earlier conditions.

Note that this is exactly the same sort of argument that people have about multiple return statements in a method. For example, if I can work out the result of a method within the first few lines (e.g. because some input is null, or empty, or zero) I find it clearer to return that answer directly than to have a variable to store the result, then a whole block of other code, and finally a return statement.