Are endless loops in bad form?

Question

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.

Answer 1

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

Answer 2

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.