views:

660

answers:

12

So I have some C++ code for back-tracking nodes in a BFS algorithm. It looks a little like this:

typedef std::map<int> MapType;
bool IsValuePresent(const MapType& myMap, int beginVal, int searchVal)
{
    int current_val = beginVal;
    while (true)
    {
        if (current_val == searchVal)
            return true;

        MapType::iterator it = myMap.find(current_val);
        assert(current_val != myMap.end());
        if (current_val == it->second) // end of the line
            return false;
        current_val = it->second;
    }
}

However, the while (true) seems... suspicious to me. I know this code works, and logically I know it should work. However, I can't shake the feeling that there should be some condition in the while, but really the only possible one is to use a bool variable just to say if it's done. Should I stop worrying? Or is this really bad form.

EDIT: Thanks to all for noticing that there is a way to get around this. However, I would still like to know if there are other valid cases.

A: 

Well, a comment saying that it is not really an infinite loop would help:

    while (true) // Not really an infinite loop! Guaranteed to return.

I do agree that it should have a condition, but this is okay in some situations (and it's not always possible or easy to make a condition).

Zifre
If the references somehow get a stray reference to something near the beginning of the chain, then it will loop forever. This is true both of the 'infinite loop' version, and of the version concocted by JaredPar. However, his version actually shows the loops intent in the conditional where yours does not
Yes! Comments always solve that problem.
Electrons_Ahoy
That's a pretty useless comment. It does not tell under what conditions it returns, nothing. A good comment might be that it returns under a specific set of conditions that are too complicated to concisely fit inside this pair of parentheses, then list the conditions. Of course, if you were to comment that clearly, you might as well CODE that clearly in the first case (which is usually the case)
Bill K
+22  A: 

I believe that there are cases where it's fine for seemingly infinite loops to exist. However this does not appear to be one of them. It seems like you could just as easily write the code as follows

while (current_val != searchVal ) {
    MapType::iterator it = myMap.find(current_val);
    assert(current_val != myMap.end());
    if (current_val == it->second) // end of the line
        return false;
    current_val = it->second
}
return true;

This seems to express the true intent of the loop better

JaredPar
+1 For clearer expression of intent and function.
Andrew Hare
stefanB
Agreed. This is easier to read, and less prone to mistakes as a result. Another thought with infinite loops -- at times it's not too bad of an idea to engineer some sort of escape in the code if it can be prone to truly "infinite" behavior. That may be in the form of checking for user input (user hits escape), or in other situations stopping after a pre-defined timeout.
Steve Wortham
+6  A: 

There are times and places for infinite loops - I am not convinced this is one of them. On the other hand, it is far from being an egregious problem here.

while (currentVal != searchVal)
{
    ...
}
return true;

One place to use them is when the process is truly indefinite - a daemon process with a monitor loop that won't terminate.

Jonathan Leffler
What if I want to make the daemon quit?
You have various choices - either break from the otherwise indefinite loop, or call an exit-like function that doesn't return. Both are effective.
Jonathan Leffler
I suppose I should say 'either break (or return) from ...'
Jonathan Leffler
Yeah but then you can't call the loop infinite ;)
@lraimbilanja: well, true - everything to do with computers is finite, and there hasn't yet been a truly infinite loop anywhere. Indeed, it isn't clear that an infinite loop can run longer than the universe, and at some point in the rather dim distant future, the universe will ... stabilize? ... and then the otherwise infinite loops will come to an end after all, a good while before reaching an infinity of time - assuming the computer doesn't break down sooner which it probably will. I used 'infinite loop' in the colloquial computerese sense of the term.
Jonathan Leffler
+5  A: 

There are situations where a construct like this makes sense:

  1. The break condition is computed within the loop
  2. There are more breaking conditions and they are all equally important
  3. You really want an endless loop ;) ..
beef2k
A: 

Stop worrying. This is not bad form if it helps to simplify the logic of the code and improve maintainability and readability. Worthwhile though to document in comments on the expected exit conditions and on why the algorithm will not slip into an infinite loop.

Stephen Doyle
As JaredPar showed, it doesn't really simplify the logic. As I said though, documenting it is definitely a good idea.
Zifre
A: 

Well, yes, but the two pages of code you have to write if you don't want your main loop to be something like while(true) is even worse form.

Electrons_Ahoy
+1  A: 

Although I've done them before, I'd vote for always trying to go for the clearer solution by using something readable, which would generally include a valid expression in the while loop--otherwise you're scanning code to look for the break.

I'm not really terrified of them or anything, but I know some people are.

Bill K
+13  A: 

My two cents is: code should be self-documenting. That is, when given a piece of code, I'd rather be able to look and tell the programmer's intent then have to read comments or trudge through the surrounding code. When I read:

while(true)

That tells me the programmer wanted an infinite loop; that the end condition couldn't be specified. This is the programmers intent in some circumstances; a server loop for instance, and that is when it should be used.

In the above code, the loop isn't meant to be forever, it has a clear end condition, and in order to be semantically clear, as others have pointed out:

while (currentVal != searchVal)

works, so the while(true) is clearly inferior and should be avoided in this instance.

Todd Gardner
+4  A: 

while(true) is used in games for the main game loop - games continually read player input, process interactions between objects and paint your screen, then repeat. This loop continues infinitely until some other action breaks out of that loop (quitting the game, finishing the level).

I tried to quickly find this main loop in the Quake 1 source code for you, but there were at least 50 occurrences of 'while(1)', as well as some written as 'for(;;)', and I wasn't immediately sure which one was the main game loop.

too much php
I wonder if this is a really good example of why NOT to use while(true)... I mean, if you only used it in the cases that don't return, you'd know which one was the game loop. Better yet, if that one said while(running), you'd know which one was the real loop and you'd be communicating to the programmer exactly how you are expecting to be stopped (otherwise is it a break? return? exit? which one will cause the correct shutdown procedure to be executed?)
Bill K
Most game loops I have seen or written are more like `while(!Exiting) { ... }`
Zifre
`while(running)` would probably have been seen as a waste of CPU cycles - checking the `running` variable would waste a cycle or two, and even more would be wasted waiting for it to be read from RAM.
too much php
+5  A: 

I agree with the other answers that there's no need for an infinite loop in this case.

However, another point might be that when you do have an infinite loop, for(;;) might be a better way to express it. Some compilers generate warnings for while(true) (condition always evaluates to false), and your intent is less clear because it looks like any other loop. Perhaps it used to say while (x == true), and you accidentally removed the x instead of the true. for(;;) says pretty clearly that this is intended to be an infinite loop. Or perhaps you intended to write something like while(t), but Intellisense in your IDE kicked in and decided to autocomplete to true.

for(;;) on the other hand, isn't something you'd ever type accidentally. (and it's easier to search for. while(true) could also be written as while(1))

Neither version is wrong, but for(;;) might be more intuitive because there is no loop condition.

jalf
+1 for explain the difference between `for(;;)` and `while(true)`
pierr
A: 

It is not uncommon to find infinite loops in embedded systems code - often surrounding finite state machines, checking peripheral chips and devices, etc.

Demi
+1  A: 

I love infinite loops as the outside control structure of a finite state machine. It's effectively a structured goto:

for (;;) {
    int c = ReadInput();
    if (c == EOF)
        return kEOF;

    switch (state) {
    case inNumber: state = HandleNumber(c); break;
    case inToken: state = HandleToken(c); break;
    case inWhiteSpace: state = HandleWhiteSpace(c);
    default:
       state = inError;
       break;
    }
    if (state == inError) ThrowError();

}

plinth