8

JavaScript supports a goto like syntax for breaking out of nested loops. It's not a great idea in general, but it's considered acceptable practice. C# does not directly support the break labelName syntax...but it does support the infamous goto.

I believe the equivalent can be achieved in C#:

    int i = 0;            
    while(i <= 10)
    {
        Debug.WriteLine(i);
        i++;
        for(int j = 0; j < 3; j++)
            if (i > 5)
            {
                goto Break;//break out of all loops
            }
    }

    Break:

By the same logic of JavaScript, is nested loop scenario an acceptable usage of goto? Otherwise, the only way I am aware to achieve this functionality is by setting a bool with appropriate scope.

P.Brian.Mackey
  • 43,228
  • 68
  • 238
  • 348
  • 4
    As always: http://xkcd.com/292/ – George Duckett Oct 24 '11 at 21:59
  • possible duplicate of [Can I use break to exit multiple nested for loops?](http://stackoverflow.com/questions/1257744/can-i-use-break-to-exit-multiple-nested-for-loops) – H H Oct 24 '11 at 22:00
  • 1
    I found this interesting article - I've never used a goto in C# myself, but I like this guy's perspective. http://weblogs.asp.net/stevewellens/archive/2009/06/01/why-goto-still-exists-in-c.aspx It describes our reactions to such *sacrilegious* ideas as (gasp) goto statements in C# pretty accurately. – David Oct 24 '11 at 22:01
  • 2
    @Dani Have you got any reason for that other than repeating what everyone else says is 'correct'? – Rob Oct 24 '11 at 22:14
  • @Rob: a choice of style I guess. you can always write you whole program with only global variables and no class and no functions other than Main. – Daniel Oct 24 '11 at 22:49
  • With `goto` it's far more easier to shoot yourself in the foot. However, if you are proficient with your gun and your aim, you can use `goto`. – Aamir Oct 25 '11 at 12:47
  • Goto can be dangerous. http://xkcd.com/292/ – Dubs Oct 25 '11 at 19:03

10 Answers10

29

My opinion: complex code flows with nested loops are hard to reason about; branching around, whether it is with goto or break, just makes it harder. Rather than writing the goto, I would first think really hard about whether there is a way to eliminate the nested loops.

A couple of useful techniques:

First technique: Refactor the inner loop to a method. Have the method return whether or not to break out of the outer loop. So:

for(outer blah blah blah)
{
    for(inner blah blah blah)
    {
        if (whatever)
        {
             goto leaveloop;      
        }
    }
}
leaveloop:    
...

becomes

for(outer blah blah blah)
{
    if (Inner(blah blah blah))
        break;
}

...

bool Inner(blah blah blah)
{
    for(inner blah blah blah)
    {
        if (whatever)
        {
             return true;      
        }
    }
    return false;
}

Second technique: if the loops do not have side effects, use LINQ.

// fulfill the first unfulfilled order over $100
foreach(var customer in customers)
{
    foreach(var order in customer.Orders)
    {
        if (!order.Filled && order.Total >= 100.00m)
        {
             Fill(order);
             goto leaveloop;      
        }
    }
}
leaveloop:    

instead, write:

var orders = from customer in customers
             from order in customer.Orders;
             where !order.Filled
             where order.Total >= 100.00m
             select order;
var orderToFill = orders.FirstOrDefault();
if (orderToFill != null) Fill(orderToFill);

No loops, so no breaking out required.

Alternatively, as configurator points out in a comment, you could write the code in this form:

var orderToFill = customers
    .SelectMany(customer=>customer.Orders)
    .Where(order=>!order.Filled)
    .Where(order=>order.Total >= 100.00m)
    .FirstOrDefault();
if (orderToFill != null) Fill(orderToFill);

The moral of the story: loops emphasize control flow at the expense of business logic. Rather than trying to pile more and more complex control flow on top of each other, try refactoring the code so that the business logic is clear.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • For your `customer.Orders` example, I would use `SelectMany`, as in `foreach (var order in customers.SelectMany(c => c.Orders)) { ... }`, which would change very little code, and reduce the nested clauses into one `foreach`. – configurator Oct 25 '11 at 18:12
  • Sometimes, the check/method is very simple and just requires one line. I would use 'gotos' in that instance, it's too much messing around. – apscience Oct 31 '11 at 11:33
  • I know this is old, but I feel like adding a "doBreakOutOfAllLoops" variable just adds noise to the code. – AnnoyinC Jun 17 '20 at 09:54
12

I would personally try to avoid using goto here by simply putting the loop into a different method - while you can't easily break out of a particular level of loop, you can easily return from a method at any point.

In my experience this approach has usually led to simpler and more readable code with shorter methods (doing one particular job) in general.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • *try* sounds too soft to me. In my experience, what you describe is *always* possible and desirable. – Sjoerd Oct 24 '11 at 22:24
  • 1
    @Sjoerd: I disagree. Sometimes putting the loop into a different method just ends up making the code that much harder to read. In particular, this will make the code harder to follow if the inner loop has no meaning when considered independent of the outer loop. Then you end up with "LoopFunc" and "LoopFuncHelper." – Brian Oct 25 '11 at 13:05
  • @Brian Then your loops are too complicated and should be refactored. See Eric Lippert's post for how to solve that case in C#. – Sjoerd Oct 25 '11 at 13:57
  • @Sjoerd: Actually, I was talking about the case where your loops were simple but needed to break out. It's perfectly possible that you have a nested loop that has only one concern rather than two concerns that can be separated. I suspect anti-goto users (such as yourself; you did say goto is *unacceptable*) would respond by either using a `bool` or by refactoring **anyhow**. If the business logic is inherently representable as a nested loop that can be broken out in the middle, I don't like the idea of wrapping a code in a function. Then, the only benefit is to replace `goto` with `return`. – Brian Oct 25 '11 at 14:11
  • @Brian As `return` is much more limited in power than `goto`, I consider that a huge benefit! When encountering a `return`, you know exactly where it leads without reading any other part of the code. When encountering a `goto`, one has to search as there is not even an indication whether it jumps forward or backwards. When encountering a `label`, one has to search which gotos jump to this place. That mental difference is huge when reading code. – Sjoerd Oct 25 '11 at 14:26
  • @Brian And again, see Eric Lippert's post for using LINQ. His final version has less lines, doesn't need comments, and separates looping and business logic. What's not to like about it?! The fact that it doesn't require a goto is just a nice side-effect. C# has the tools to write very readable code, so why insist on being able to write harder to understand code? – Sjoerd Oct 25 '11 at 14:36
  • @Sjoerd: The thing I don't like about his LINQ code is that it's not appropriate when you're trying to work with array directly. Usually LINQ is much clearer than working with an array, but not when the business logic is actually talking about arrays rather than collections (i.e., when A[X] means "the element of A at position X" and the position of that element is itself inherently meaningful). I think it's incredibly rare for a goto to be more readable than an alternative, but it does happen. – Brian Oct 25 '11 at 14:49
11

Let's get one thing straight: there is nothing fundamentally wrong with using the goto statement, it isn't evil - it is just one more tool in the toolbox. It is how you use it that really matters, and it is easily misused.

Breaking out of a nested loop of some description can be a valid use of the statement, although you should first look to see if it can be redesigned. Can your loop exit expressions be rewritten? Are you using the appropriate type of loop? Can you filter the list of data you may be iterating over so that you don't need to exit early? Should you refactor some loop code into a separate function?

slugster
  • 49,403
  • 14
  • 95
  • 145
5

IMO it is acceptable in languages that do not support break n; where n specifies the number of loops it should break out.
At least it's much more readable than setting a variable that is then checked in the outer loop.

ThiefMaster
  • 310,957
  • 84
  • 592
  • 636
2

I believe the 'goto' is acceptable in this situation. C# does not support any nifty ways to break out of nested loops unfortunately.

tier1
  • 6,303
  • 6
  • 44
  • 75
  • `return` will break out of nested loops. And see the top answers for suggestions for rewrite to improve code readability (and 'accidently' avoiding the goto at the same time). – Sjoerd Oct 24 '11 at 23:05
2

It's a bit of a unacceptable practice in C#. If there's no way your design can avoid it, well, gotta use it. But do exhaust all other alternatives first. It will make for better readability and maintainability. For your example, I've crafted one such potential refactoring:

void Original()
{
    int i = 0;            
    while(i <= 10)
    {
        Debug.WriteLine(i);
        i++;
        if (Process(i))
        {
            break;
        }
    }
}

bool Process(int i)
{
    for(int j = 0; j < 3; j++)
        if (i > 5)
        {
            return true;
        }
    return false;
}
Jesse C. Slicer
  • 19,901
  • 3
  • 68
  • 87
0

anonymous functions

You can almost always bust out the inner loop to an anonymous function or lambda. Here you can see where the function used to be an inner loop, where I would have had to use GoTo.

private void CopyFormPropertiesAndValues()
{
    MergeOperationsContext context = new MergeOperationsContext() { GroupRoot = _groupRoot, FormMerged = MergedItem };
    // set up filter functions caller
    var CheckFilters = (string key, string value) =>
    {
        foreach (var FieldFilter in MergeOperationsFieldFilters)
        {
            if (!FieldFilter(key, value, context))
                return false;
        }
        return true;
    };
    // Copy values from form to FormMerged 
    foreach (var key in _form.ValueList.Keys)
    {
        var MyValue = _form.ValueList(key);
        if (CheckFilters(key, MyValue))
            MergedItem.ValueList(key) = MyValue;
    }
}

This often occurs when searching for multiple items in a dataset manually, as well. Sad to say the proper use of goto is better than Booleans/flags, from a clarity standpoint, but this is more clear than either and avoids the taunts of your co-workers.

For high-performance situations, a goto would be fitting, however, but only by 1%, let's be honest here...

FastAl
  • 6,194
  • 2
  • 36
  • 60
  • That won't compile. You can't assign a lambda to a `var` variable. The compiler doesn't know what type the lambda should be converted to. You're better off just using a local function here anyway, rather than an anonymous function. – Servy Jul 17 '18 at 21:46
  • @servy - you caught me, I didn't try to compile it. I've been programming in Haskell for the past few months, so perhaps my C# is rusty ;-) – FastAl Jul 20 '18 at 19:54
0

I recommend using continue if you want to skip that one item, and break if you want to exit the loop. For deeper nested put it in a method and use return. I personally would rather use a status bool than a goto. Rather use goto as a last resort.

Gerhard Powell
  • 5,965
  • 5
  • 48
  • 59
-1
int i = 0;                 
while(i <= 10)     
{         
    Debug.WriteLine(i);         
    i++;         
    for(int j = 0; j < 3 && i <= 5; j++)             
    {
        //Whatever you want to do
    }
 }      
rare
  • 134
  • 4
-2

Unacceptable in C#.

Just wrap the loop in a function and use return.

EDIT: On SO, downvoting is used to on incorrect answers, and not on answers you disagree with. As the OP explicitly asked "is it acceptable?", answering "unacceptable" is not incorrect (although you might disagree).

Sjoerd
  • 6,837
  • 31
  • 44
  • 2
    The language supports it. In this sense, it's certainly not unacceptable. By what basis is something "acceptable" or "unacceptable" to you? – Andrew Flanagan Oct 24 '11 at 22:00
  • @Andrew The OP already said it was possible, and explicitly asked whether it was acceptable. – Sjoerd Oct 24 '11 at 22:06
  • 2
    Unacceptable? Never seen a need for one, but "unacceptable" is too much. – Alexei Levenkov Oct 24 '11 at 22:08
  • @Henk Read "Goto considered harmful" by Dijkstra. – Sjoerd Oct 24 '11 at 22:08
  • 3
    @Sjoerd - read [“Considered Harmful” Essays Considered Harmful](http://meyerweb.com/eric/comment/chech.html) – H H Oct 24 '11 at 22:13
  • It's also used on answers which don't provide any context or reasoning behind it. – Rob Oct 24 '11 at 22:15
  • @Henk That link does not disagree with Dijkstra - it does not even try to! It just disagrees with all copy-cats that try to mimic the success of the original article. – Sjoerd Oct 24 '11 at 22:16
  • @Rob There is a reason mentioned: "Just wrap the loop in a function and use return." – Sjoerd Oct 24 '11 at 22:18
  • @Sjoerd Sure, but that doesn't mean it's `unacceptable`. Is it unacceptable to use a loop instead of recursion? Just because there's an alternative doesn't mean its unacceptable – Rob Oct 24 '11 at 22:19
  • 1) It's not Dijkstra's title, 2) Dijkstra was talking in a different era about very different languages. He was talking about GOTO based languages, not about the occasional use in a structured language. – H H Oct 24 '11 at 22:23
  • 2
    @Rob So we disagree. But it's a valid answer to the question, and there is a reason. Therefor: no need to downvote. – Sjoerd Oct 24 '11 at 22:26
  • 1
    @Sjoerd You gave an alternative implementation, not a reason why it's better, or why goto shouldn't be used. – Rob Oct 24 '11 at 22:30
  • @Henk The reasons Dijkstra mentioned, are still valid in today's languages. In short: a goto is too powerful - whenever one sees a goto or a label, one has to check all code to see what is happening. That reasoning is still valid. In fact, it's even more valid in today's languages with alternatives, as the question "why goto?" pops up and one start looking for reasons that aren't there. – Sjoerd Oct 24 '11 at 22:31
  • 1
    Sjoerd, by your own reasoning anyone who considers a goto (even slightly) acceptable should downvote this answer. – H H Oct 24 '11 at 22:31
  • @Henk I can't follow your reasoning. – Sjoerd Oct 24 '11 at 22:33
  • I wouldn't downvote it, but who would not be curious 'why' it's unacceptable? You are not our mother, or boss, or teacher, so if you write like that, you are just inviting haters. And maybe they are right about hating your answer ;-) The least you could do is link to Dijkstra's article _in your answer_. I know you believe it since you mentioned it. And people tend to get defensive enough when their actual mothers act like their mothers, let alone complete strangers. (but I have not seen my mother in a long time, maybe you are you my mother!!) – FastAl Jul 20 '18 at 19:58