Is using a 'goto' statement bad?

EDIT:

How bad is the goto statement really, and why?

It depends on the exact situation. I can't remember any time where I found it made the code more readable than refactoring. It also depends on your personal view of readability - some people dislike it more than others, as is clear from the other answers. (As a point of interest, it's widely used in generated code - all of the async/await code in C# 5 is based on effectively a lot of gotos).

The problem is that situations where goto tends to be used tend to be the kind of situations where refactoring aids things anyway - whereas goto sticks with a solution which becomes harder to follow as the code gets more complicated.

Is there a more effective way to break the main loop than using the 'goto' statement?

Absolutely. Extract your method out into a separate function:

while (ProcessValues(...))
{
    // Body left deliberately empty
}

...

private bool ProcessValues()
{
   for (int i = 0; i < 15; i++)
   {
       // Do something
       return false;
   }
   return true;
}

I generally prefer doing this over introducing an extra local variable to keep track of "have I finished" - although that will work too, of course.


I'm going to strongly disagree with all of the other answers here. The code you present using goto has nothing wrong with it. There is a reason C# has a goto statement, and it is precisely for these types of scenarios which you describe.

goto simply has a negative stigma because in 1970s and prior people would write horrible, completely unmaintainable code where control flow jumped all over the place because of goto. C#'s goto does not even allow transitioning between methods! Yet there is still this irrational stigma against it.

In my opinion, there is absolutely nothing wrong with using a "modern" goto to break out of an inner loop. The "alternatives" people offer always end up being more complicated and harder to read.

Methods are generally supposed to be reusable. Making a whole separate method for the inner part of a loop, that will only ever get called from that one location, and where the method implementation may end up being at some distant location in the source code, is not an improvement.


I sometimes use "goto" and I found it looks good, like example above;

bool AskRetry(Exception ex)
{
  return MessageBox.Show(you know here...) == DialogResult.Retry;
}

void SomeFuncOrEventInGui()
{
  re:try{ SomeThing(); }
  catch (Exception ex)
  {
    if (AskRetry(ex)) goto re;
    else Mange(ex); // or throw or log.., whatever...
  }
}

I know you can do same thing recursively, but who cares it just works and I use.


How bad is the goto statement really, and why?

It's really bad for all the normal reasons given. It's prefectly fine when emulating labeled loops in languages that don't support them.

Replacing it with functions will in many cases scatter logic that really should be read as the same unit. This makes it harder to read. Nobody likes to follow a trail of functions that don't really do anything until at the end of the journey, when you have somewhat forgotten where you started from.

Replacing it with booleans and a bunch of additional ifs and breaks is just really clunky and makes it harder to follow real intentions, like any noise.

In java (and javascript), this is perfectly acceptable (labeled loops):

outer: while( true ) {
    for( int i = 0; i < 15; ++i ) {
        break outer;
    }
}

In C#, it looks like the very close equivalent isn't:

while( true ) {
   for (int I = 0; I < 15; I++) { 
       goto outer;
   }
}
outer:;

Because of the word goto, which has a psychological effect of making people drop all their common sense and make them link xkcd regardless of context.

Is there a more effective way to break the main loop than using the 'goto' statement?

In some cases there isn't, which is why the other languages provide labeled loops and C# provides goto. Note that your example is too simple and it makes the work-arounds not look too bad because they're tailored to the example. In fact, I could just as well suggest this:

   for (int I = 0; I < 15; I++) {
       break;
   }

How about this:

int len = 256;
int val = 65536;

for (int i = 0; i < len; i++)
{
    for (int j = 0; j < len; j++)
    {
        if (i + j >= 2 * val)
        {
            goto outer;
        }
        val = val / 2;
    }
}
outer:;

Does this still look good to you:

int len = 256;
int val = 65536;

for (int i = 0; i < len; i++)
{
    if (!Inner(i, ref val, len))
    {
        break;
    }
}

private bool Inner(int i, ref int val, int len)
{
    for (int j = 0; j < len; j++)
    {
        if (i + j >= 2 * val)
        {
            return false;
        }

        val = val / 2;
    }

    return true;
}

Tags:

C#

Loops

Goto

Break