[Proposal] Labeled loops like in Java

[AustinBryan]

Currently the cleanest way to break out of a nested loop is this:

for (int i = 0; i < 5; i++)
    for (int j = 0; j < 3; j++)
        if (i * j == 10) 
            goto outer;

outer: {}

If all we wanted to do was break out of the loops, we have to do outer: {}, which is sort've clunky. What we could do instead is use labeled loops like Java, so we can do this:

outer: for (int i = 0; i < 5; i++)
    inner: for (int j = 0; j < 3; j++)
        if (i * j == 10) 
            break outer;

And instead of:

int j = 0;

for (int i = 0; i < 5; i++)
    inner: for (; j < 3; j++)
        if (i * j == 10) 
            goto inner;

We can just do:

for (int i = 0; i < 5; i++)
    inner: for (j = 0; j < 3; j++)
        if (i * j == 10) 
            continue inner;

The alternative is cleaner, more intuitive, and simpler.

I saw another post where they suggest doing break 1; to break the first loop, but that doesn't refactor well if the number of loops changes, you have to carefully make sure the numbers are correct.

[iam3yal]

@AustinBryan Related #869

[Joe4evr]

Also relevant: dotnet/roslyn#5883

[jnm2]

I like labeled break much better. Also would continue outer; work?

[AustinBryan]

@jnm2

Also would continue outer; work?

Yes, it should. I just tested it out on Java and it works for that so it should for C#.
Labeled breaks are the cleanest, most straight foreward, and least error prone way to do manage nested loops.

[quinmars]

In the last three years I needed only once a goto statement to leave a nested foreach loop. Later on I replaced it by a single foreach loop thanks to LINQ. I don't realy understand the absolute avoidance of goto. There are rare cases (and they are very rare) where it comes in handy to use it and there is nothing wrong about it.

[iam3yal]

@AustinBryan I think that your second case was rejected, you can check the post @Joe4evr linked.

[Neme12]

So break outer would essentially be the same as goto except that it would jump after the labeled statement? I don't see much value in that.

[jnm2]

@Neme12 It's clearer about what's going on. More symmetrical with continue outer and guaranteed to be an acceptable kind of goto.

[AustinBryan]

@Neme12 I mean yes, except it reflects more on what we want to do. In this example, what I want to do is break outer, not goto some random empty statement. continue outer is much simpler than the alternative that uses goto

[DavidArno]

Currently the cleanest way to break out of a nested loop is this:

I disagree. the cleanest way is to exit the function, avoiding goto and break:

void LoopForTheFunOfIt()
{
    for (int i = 0; i < 5; i++)
        for (int j = 0; j < 3; j++)
            if (i * j == 10)
                return; 
}

If you end up with code that needs a goto, don't reach for labelled breaks, reach for the refactoring tools and split out a new method.

[theunrepentantgeek]

If you end up with code that needs a goto, don't reach for labelled breaks, reach for the refactoring tools and split out a new method.

I kind of agree - in code reviews, I usually see nested loops as a code smell indicating a need to break out another method.

[iam3yal]

@theunrepentantg

I usually see nested loops as a code smell indicating a need to break out another method.

Wouldn't that be an extreme? I mean, indeed, sometimes it's the case but saying that you usually tend to find nested loops a code smell when a code smell should indicate that there's a problem with the design not with how the code is structured but more importantly this statement shouldn't be taken as rule of thumb because what's applicable to one codebase might not be applicable to another, one might be more imperative in nature whereas the other can be more declarative and so it really depends.

Refactoring too much can also lead to readability and maintainability issues.

[theunrepentantgeek]

Refactoring too much can also lead to readability and maintainability issues.

I totally agree - readability, maintainability - and all the 'ilities (scalability, supportability, etc) are very important.

Nonetheless, I stand by the statement: Usually nested loops are a design smell.

a code smell should indicate that there's a problem with the design not with how the code is structured

I don't understand this - how can well-designed code be structured poorly? Design covers all scales - from the macro architecture (components, services, etc) to the micro (algorithm choice, method signatures, local variable naming, etc).

[iam3yal]

don't understand this - how can well-designed code be structured poorly?

@theunrepentantgeek I guess that this is where we disagree, you seems to think that nested loops is an indication of a code smell whereas I think that it might be the case but you don't always need to refactor nested loops especially in this case where the language can be improved a bit and make the goto statement more convenient to work with so just because some people decide to ban it or don't see any use for it doesn't mean people who do shouldn't use it and instead introduce methods into their codebases that wouldn't really improve anything.

You can structure the code in many different ways, you can structure it through a series of imperative statements, you can use a more procedural approach, you can use a more declarative approach and apply all kinds of functional programming techniques and yet the design can be broken so just because you refactored something into its own method and you gave it a name and it looks good doesn't mean you improved the design, in fact, it might mean the opposite but really context is important here and in this case the kind of codebase you work with should be considered before going out and tell people what they should or shouldn't do.

Now, some people would consider their codebase state of the art because it's refactored and follow awesome set of principles and whatnot only to find out that it's actually poorly designed.

[DavidArno]

...you seems to think that nested loops is an indication of a code smell...

Such code runs counter to the "code should describe the intent; not the mechanism" principle of easy-to-reason (or "reasonable") code. So if I write code like:

var found = false; 
for (var i = 0; i < range1; i++)
for (var j = 0; j < range2; j++)
    if (i * j == 10)
    {
       found = true;
       goto outer;
    }
outer: {}

I'm purely expressing the mechanism. The code is highly unreasonable. Therefore the next thing I'm likely to do is add a comment to explain its intent:

// loop through the two ranges and report if they contain factors of 10
var found = false; 
for (var i = 0; i < 5; i++)
for (var j = 0; j < 3; j++)
    if (i * j == 10)
    {
       found = true;
       goto outer;
    }
outer: {}

Now I have the intent, but I still have the mechanism there too. It's a mess. I'm not sure If it's a code smell, but it certainly is ugly and difficult to read. What I want is the intent only:

var found = RangesContainFactorsOfTen(5, 3);

The code is now trivially easy to reason. And when I want to understand the mechanism, I head off to that method:

private bool RangesContainFactorsOfTen(int range1, int range2)
{
    for (var i = 0; i < range1; i++)
    for (var j = 0; j < range2; j++)
        if (i * j == 10)
            return true;

    return false;
}

So I've achieved code that is far easy to reason and read, I've removed the comment and I've avoided the need for a goto or labelled break (which is just syntactic sugar for a goto).

C# is already complicated enough with a growing number of features with every release. Those new features should be focused on the "pit of success", ie making it easier to write good code. Labelled breaks are the opposite: they just offer another way, in addition to the existing one, of writing bad code. So they they have no place in future versions of the language in my view.

[CyrusNajmabadi]

@CyrusNajmabadi My argument was that there is an impedence mismatch, and the need for an analyzer was the smallest part of that argument. I'm not saying analyzers are bad.

Understood. Sorry for trivializing your argument hte away i did.

[CyrusNajmabadi]

Jumping back a bit. I can see the argument for why labeled-loops seem to be desirable. They have the following pros (depending on your perspective).

1. They seem syntactically/semantically 'attached' to the construct of relevance.
2. They are not as free-wheeling as 'unrestricted gotos'.
3. They are a better 'fit' since the primary reason to jump backwards is for looping. So better to just tie this to the loops instead of just tying it to 'arbitrary jumps'.
4. They are 'clear' (depending on your perspective) because continue/break already make sense for a loop, and people already have to understand it. So this is just saying which loop to continue/break form.

Overall, these are a cogent and clear argument for why such a feature at least "makes sense" in isolation. And, frankly, i'd likely be swayed by this for C# v1 to have this instead of having hte arbitrary-goto feature.

Clearly the above (and likely other stuff i missed) is sufficient for some people to really want this even for a v8+ release. I can accept and respect that.

--

At this point it really comes down to: Is there an LDM member that thinks the above is sufficient? Or is the alternative we have today (gotos + optional-analyzers) reasonably sufficient (and cost effective) to just stick with the status-quo. My personal opinion is "the status quo is fine". However, not everyone may share htat opinion. We'll just have to see if someone wants to champion this.

[jnm2]

@CyrusNajmabadi All good, thanks for listening!

Ok... so if you'd name the loop... why can you not just label the appropriate jump points if you were to use 'gotos'. I feel like i'm not getting your argument here. :)

goto next_widget;...next_widget: } versus processing_widgets: for...continue processing_widgets;?

The latter is better documentation of the loop declaration and feels right. The former feels like a hack, is harder to set up and read, and doesn't contain richer information. Possibly less rich.

[Korporal]

Labels are just a means to an end a way of identifying the loop to which the break or continue is to be applied. One could avoid labels altogether with a construct like:

break (0); // current loop

or

continue (2); // loop outside the loop outside the current loop

I'm not advocating that of course but do want to point out that a label is not the only way to provide the information to the break or continue.

[CyrusNajmabadi]

The latter is better documentation of the loop declaration and feels right. The former feels like a workaround with low information.

It's very interesting how different people see these things. I already view a loop as incredibly low-level and imperative. To me your next_widget example is so much clearer and fits far better with how i think about the imperative nature of loops in C-style languages :D

To each their own.

[jnm2]

Huh. It's probably also worth pointing out that I'd only find myself doing this if I was inlining for very specific hot paths. Otherwise I'd use .FirstOrDefault. And because the goto thing feels so fragile, half the time I just use a flag which is probably not more readable than the goto.

[HaloFour]

@jnm2

The latter is better documentation of the loop declaration and feels right.

That's my problem with it. It documents the loop declaration, not the reasons for wanting to leave the loop. If you have two reasons for wanting to break, they'd both be accomplished with break processing_widgets which doesn't tell you anything.

[jnm2]

@HaloFour That's consistent with how I use continue and break today (not to mention return).

[CyrusNajmabadi]

@HaloFour That's consistent with how I use continue and break today (not to mention return).

IN that case, i think it's totally reasonable to just do the same with "goto" :)

At this point, i think the arguments have been exhausted. I think i totally get your position @jnm2 . it's consistent and not incorrect in any fashion. It will be up to an LDM member to see if they want to champion.

argue:
    MakePoints();
    MakeCounterPoints();
    goto argue;

Seems totally clear to me :D

[Neme12]

@Korporal

So although the benefit of break label may be relatively small if it's cost is correspondingly small then surely it's worthy of serious consideration on that basis? This whole concept, used in software all the time is often informally described by the term "low hanging fruit".

I suspect this is a small change to the C# compiler code base.

Please note that when we talk about cost, it's not just "how hard is this to implement in the compiler"?
The feature simply may not be worth the added complexity to the language. For example, every feature could potentially clash with another feature you might want to add in the future. But even if that is not a concern, you cannot add new language features willy-nilly just because they might be marginally useful in a few scenarios. You're adding a new feature for every developer or beginner to learn. You're adding more ways to do the same thing and forcing people to understand each style. Language design is really unlike product or even API design. It's simply not a matter of "how hard can this be"? The right question is: Does this provide enough value to justify the added complexity? In my opinion, the answer is no.

I'm also really tempted to fork the repo, implement the change and submit a pull request and be done with this rather simple change.

You're welcome to do that for you own use but please know that if you plan to submit a PR to Roslyn, you likely won't even be allowed to keep it open or have a feature branch unless this feature is championed. If you find it fun to experiment with this or just implement in case it would get considered in the future, that's awesome but don't be disappointed if that doesn't happen.

[Korporal]

I certainly would be interested in making such a change but there's clearly a ramp up time and we know the team already has the expertise for this. What I was trying to convey is that it is probably within my abilities despite my limited knowledge of roslyn, unlike some other changes.

I have no idea how the roslyn design ultimately works but one could possibly just implement some form of transformation that transforms this:

            int x = 0;

            my_loop:

            while (true)
            {
                if (x > 10)
                    continue my_loop;

                if (x == 30)
                    break my_loop;
            }

into this:

            int x = 0;

            var my_loop_flag = true;

            my_loop:

            if (my_loop_flag)
            {
                while (true)
                {
                    if (x > 10)
                    {
                        my_loop_flag = true;
                        goto my_loop; // continue my_loop
                    }


                    if (x == 30)
                    {
                        my_loop_flag = false;
                        goto my_loop; // break my_loop
                    }
                }
            }

which is why I described this as syntactic sugar earlier.

[CyrusNajmabadi]

I have no idea how the roslyn design ultimately works but one could possibly just implement some form of transformation that transforms this:

Yup. That's how roslyn works already today. Indeed, as IL has no concept of loops at all, all loops in roslyn are ultimately "lowered" into just equivalent forms with appropriate "gotos". So you could certainly implement this feature in such a manner.

[Rekkonnect]

I took some time to create the spec for this feature here, if that helps

[333fred]

This feature is not championed by any member of the LDM. Until that happens, there will be no progress here.

[Richiban]

There's a use case I've come across a few times now that's not mentioned here: needing to use break; to leave a loop from inside a switch statement.

Since pattern matching was introduced to the language and keeps improving I find myself using them more and more, but a number of times I've found myself having to write this annoying kludge:

var @break = false;

foreach (var c in input)
{
    switch (c)
    {
        case 'a':
            // snip
            break;
        case 'b':
            // snip
            break;
        case > '0' and < '9':
            // snip
            break;
        case 'c':
            // What I really want to do here is break out of the loop, not the switch statement
            if (anyDigits)
            {
                @break = true;
            }
            
            break;
        default: @break = true; break;
    }
    
    // Now I have to check my boolean to break myself
    if (@break) break;
}

It's possible this particular usecase could get its own feature, such as break@foreach;, but if we're going to go there we might as well include nested loops as well.

[Rekkonnect]

In such scenarios, there can also be the possibility of permitting code like break foreach, which binds to the innermost foreach loop, if it exists, and refers to breaking that, recursively applying for all other nested elements. This would permit syntax like the following:

• break for;
• continue foreach;

There is a huge catch with this syntax, though. In a future version, these keyword combinations could refer to some other convenience language construct, which is far from unlikely, considering the words' human language meanings. And, that syntax is too niche to find reasonable grounds.

[TonyValenti]

I have always appreciated VB's "exit for" construct. I do wish c# allowed an easier way to deal with this.

[Rekkonnect]

That's one of the very few cases where C# can be jealous of VB. I'd love that number to be reduced to 0 :)

[HaloFour]

It's handy, except when you have nested loops of the same type, which is a situation I've run into in VB and VB.NET more than once. If either approach were to be considered I would prefer labeled loops which doesn't have that limitation. That said, it's a feature I very rarely find myself using in Java.