Infinite Loops and Early Return Statements_问答_开发者

I have a simple console application that outputs a menu and waits for user input. After performing the appropriate action, the entire process repeats. The program exits when a specific string is entered. This is implemented with an infinite loop and an early return statement:

int main()
{
    while (true)
    {
        OutputMenu();

        string UserChoice;
        cin >> UserChoice;

        // ...

        if (UserChoice == "exit") return 0;
    }
}

According to my teacher, it's bad practice to use an infinite loop and hack my way out of it with a return statement. He suggests something like the following:

int main()
{
    bool ShouldExit = false;
    while (!ShouldExit)
    {
        OutputMenu();

        string UserChoice;
        cin >> UserChoice;

        // ...

        if (UserChoice == "exit") ShouldExit = true;
    }

    return 0;
}

Is it really a bad idea to use an infinite loop and an early return statement?
If so, is there a technical reason or is it just bad practic开发者_Python百科e?

This might be one of those rare cases where do...while is appropriate. I avoid adding extra boolean state variables unless they genuinely make the code clearer.

int main()
{
    string UserChoice;
    do
    {
        OutputMenu();

        cin >> UserChoice;

        // ...

    } while (UserChoice != "exit");
}

However, for a user input loop I would usually make a function that returns whether or not the input was successful. As it stands the code could easily end in an infinite loop if cin closes.

E.g.

bool GetNonExitInput( std::istream& in, std::string& s )
{
    OutputMenu();
    in >> s;

    return in.good() && s != "exit";
}


int main()
{
    std::string UserChoice;

    while (GetNonExitInput(std::cin, UserChoice))
    {
        // ...
    }
}

Really either is fine, but you need to do what your professor wants. You'll find it is the same in industry as well. Some companies may have a coding standard that dictates curly braces go on a new line, while others want them to start on the line that begins the block. There is no real reason to prefer one over the other, so it is best to just go with what the lead wants.

The only difference between these two approaches is that in the second approach you can still do something after you exit the while loop, while in the first approach, you're returning from the function itself; you can do nothing after the while.

However, I would suggest this simple code : instead of maintaining a variable, you can also use break like this:

while (true)
{
       //your code

       if (UserChoice == "exit") 
            break;

       //your code
}

The variable ShouldExit is not needed anymore!

Depends on the language. If you're writing in C, then the "one entry, one exit" philosophy makes sense -- you want one place where you're cleaning up resources used by a function so that you don't have a chance of forgetting later. If you're in C++, then you should be using RAII for cleanup anyway, in which case I completely disagree with your teacher. Use returns as needed in order to make the code as clear as possible.

(Though I would use for (;;) instead of while (true) in C++ to generate the infinite loop)

With the controlled variable you would be able to handle exit conditions (code after the while) before exiting the function.

In my opinion both ways are fine, but the second one is "prettier".

In programing it is important to write the code in the easiest way you can think of, and make it simple for other programmers to understand your code if you'll be replaced or for any other reason.

There is no complexity issue involved with your two codes so they are both fine as I said, but the think I don't like about the first code is the use of 'return' statment without any real need of 'return' statment here.

There is another way writing this code, better then your way (in my opinion), but not better as your teacher's one.

int main()
{
    bool ShouldExit = false;
    while ( true )
    {
        OutputMenu();

        string UserChoice;
        cin >> UserChoice;

        // ...

        if (UserChoice == "exit") break;
    }
}

Another main reason why I don't like your first code and my code above is because of the use of infinite loop, when you make yourself used to infinite loops it is just a matter of time until you will make your more complicated programs with major bugs in it.

Again - all of the things I wrote are in my opinion only and not gospel truth.

Rotem

Technically, there's not much in it, as long as there's no code you're skipping over via the use of the return.

However, your teacher's suggestion is more readable, if only because of the obvious meaning of "ShouldExit".

I think what your teacher means that the exit condition can easily maintained. This because of code cleanup after the while loop. If you will do a hard return then everything after the while loop will not be executed. This can prevented by using a break instead of return.

int main()
{
 //create a file

 while (true)
 {
  OutputMenu();

  string UserChoice;
  cin >> UserChoice;

  //write UserChoice to file

  // ...

  if (UserChoice == "exit") return 0;
 }

 //close file 
}

//close file will then not be executed!

The break statement is specifically for exiting loops.

I would usually prefer what your teacher suggested, simply because it's easier to read and understand what are the conditions to stop the loop.
If you have an infinite loop with a return statement it's a little bit more difficult for someone who didn't write the code to go through the code and figure out when the program will hit a return statement.

Also I usually don't like early returns in general because it's easy for someone maintaining the code to introduce bugs, for example:

int main()
{
    // code added by some other programmer:
    importantInitialization();

    while (true)
    {
        OutputMenu();

        // code added by some other programmer:
        Something *st = new Something();

        string UserChoice;
        cin >> UserChoice;

        // ...

        if (UserChoice == "a") runA();
        else if (UserChoice == "b") runB();
        else if (UserChoice == "c") runC();
        else if (UserChoice == "d") runD();
        else if (UserChoice == "exit") return 0;
        else if (UserChoice == "help") showHelp();

        // code added by some other programmer:
        delete st; // this would not run on the last loop
    }

    // code added by some other programmer:
    importantCleanUp(); // this would never run 
}

of course that in this particular case it's easy to see the problems, but when maintaining a more complicated function you can see how an early return statement might make it even more prone to lack-of-attention bugs like this.

I think the while(true) with break is best for a few reasons. Introducing a variable to store the exit condition is error prone, more variables means more can go wrong. Also the break statement is meant specifically for breaking out of loops. Lastly, contrary to for(;;), while(true) is clean, readable, and concise, where for(;;) is trying to be clever for no good reason.

For an added point, to enhance readability and comprehension put the exit condition(s) nearest to the top of the loop as possible:

while (true) {

    OutputMenu();

    string UserChoice;
    cin >> UserChoice;

    if (UserChoice == "exit")
        break;

    // process other options here
}

A routine which uses a local flag is, in terms of state analysis, equivalent to the same code, copied out twice, with one copy corresponding to the flag being true, and the other copy being equivalent to the flag being false, and any code which changes the state of the flag jumping between them. If there are n flags, the equivalent would be 2^n copies of the code (though if some flags are mutually exclusive, some of those may be unreachable and irrelevant).

While there are certainly times that flags are the most practical way to do things, they add complexity to the code. When the complexity is really necessary, flags may be the cleanest way to provide it. When there's a clean and practical way to write code which avoids the flags, one should do so. There are certainly times when it may be unclear whether it's better to use or avoid a flag (e.g.

  flag = condition_which_must_be_tested_here();
  action_which_will_disturb_the_condition();
  if (flag)
    do_something();
  else
    do_something_else();

versus

  if (condition_which_must_be_tested_here())
  {    
    action_which_will_disturb_the_condition();
    do_something();
  }
  else
  {    
    action_which_will_disturb_the_condition();
    do_something_else();
  }

but in cases where no code can be written without a flag and without having to duplicate anything, such a version is generally preferable.

I'm philosophically opposed to while(true). It means "loop forever" and you never really want to loop forever.

On the other hand I'm also philosophically opposed to boolean variables that merely record state that can be found out in other ways. There's a bug in waiting in that it may not always be correctly synchronised with the state it is supposed to reflect. In this case, I'd prefer code like:

int main()
{
    string UserChoice = "not started"; // or empty string
    while (UserChoice != "exit")
    {
        OutputMenu();

        string UserChoice;
        cin >> UserChoice;

        // ...

    }

    return 0;
}