Discussion: C# Break nested loop

[jcouv]

@Danthekilla commented on Thu Aug 31 2017

In C# it would be fanstastic to have a language feature (probably syntactic sugar) that lets you break nested loops.

Currently these are the 2 ways you can break out of nested loops:

var foundValue;
for (int x = 0; x < xMax; x++)
{
    for (int y = 0; y < yMax; y++)
    {
        foundValue = GetValue(x, y);
        if (foundValue == valueToCompare)
            goto ENDOFLOOPS;
    }
}
ENDOFLOOPS:

var foundValue;
bool shouldBreak;
for (int x = 0; x < xMax; x++)
{
    for (int y = 0; y < yMax; y++)
    {
        foundValue = GetValue(x, y);
        if (foundValue == valueToCompare)
        {
            shouldBreak = true;
            break;
        }
    }
    if (shouldBreak)
        break;
}

I would like to propose the ability for a break to have a value for how many loops to break out of. Like so:

var foundValue;
for (int x = 0; x < xMax; x++)
{
    for (int y = 0; y < yMax; y++)
    {
        foundValue = GetValue(x, y);
        if (foundValue == valueToCompare)
            break 2;
    }
}

Not only is it slightly smaller, but I think easier to read.

Anyone have any thoughts on the matter?

[DavidArno]

@Danthekilla,

You forgot the existing third way:

(bool found, ValueType value) Foo()
{
    for (int x = 0; x < xMax; x++)
    {
        for (int y = 0; y < yMax; y++)
        {
            var foundValue = GetValue(x, y);
            if (foundValue == valueToCompare) return (true, foundValue);
        }
    }
    return (false, default);
}

Small focused functions are your friend. But if you want long functions, as you say, there is already two ways of achieving this goal. So I'd see yet another way of achieving it as really bringing nothing to the language.

[jnm2]

Sometimes it's a loop and a switch and coming up with a name for it is super arbitrary and counter-intuitive.

[iam3yal]

I'm not in favour of this but I think the following is better goto ENDOFLOOPS when foundValue == valueToCompare; I know, no one "likes" the goto statement but anyway, we might get a goto and continue expressions so the following would be possible:

var foundValue;
for (int x = 0; x < xMax; x++)
{
    for (int y = 0; y < yMax; y++)
    {
        foundValue = GetValue(x, y);
	foundValue == valueToCompare ? goto ENDOFLOOPS : continue;
    }
}
ENDOFLOOPS:

[Logerfo]

I'd prefer to see something like Java Branching Statements, as once pointed out by @HaloFour at #176 (comment)

[jnm2]

goto is perfectly reasonable if you only go to points that can already be jumped to by another kind of branching statement like break. Same with goto case.

[iam3yal]

@Logerfo, Indeed, it makes things clear than having something like break <number>.

[benaadams]

Inline returns slow down loops until .NET Core 2.1 dotnet/coreclr#13314 and functions with a loop generally won't inline (without forcing); so wouldn't be so great for very hot loops; though ok for general loops.

[bondsbw]

break <number> doesn't refactor as well. Adding another loop or removing a loop requires carefully reading back through the code and reworking such statements. Otherwise, it can result in jumping to an unintended point.

[jnm2]

@bondsbw Not to mention copy/paste.

[JarrydSemmens]

This is simple, short and elegant. Goto's require maintenance and their excessive use results in spaghetti code. Extra temp variables hanging around is just junk code that can go wrong.

Break = n; in an n deep loop would create short, concise code that cannot be confused for anything else.

[HaloFour]

I agree with some of the other posters, I don't like the use of an integer indicating the number of levels. If any enhancements to break or continue are considered for nested loop scenarios I'd prefer the labeled loop syntax used by Java.

String foundValue = null;
OUTERLOOP:
for (int x = 0; x < xMax; x++)
{
    for (int y = 0; y < yMax; y++)
    {
        foundValue = GetValue(x, y);
        if (foundValue == valueToCompare)
            break OUTERLOOP;
    }
}

This makes it much clearer exactly where the break/continue statement will continue execution.

[orthoxerox]

I agree that breaking from a named loop is the best option. I think there was a proposal of mine asking for that. Or was it left in the roslyn repo?

[iam3yal]

@jnm2 It's better to have an "arbitrary" label and know what it is than having a number and starting to wonder what it means, sometimes.

[jnm2]

@eyalsk Exactly my point. 😄

[jnm2]

@eyalsk I don't remember preferring numbers but I'm happy to update my opinion if I did write that somewhere. I'm not seeing it.

[iam3yal]

@jnm2 I was referring to this comment.

Sometimes it's a loop and a switch and coming up with a name for it is super arbitrary and counter-intuitive.

Anyway, deleted my comment.

p.s. I might misinterpreting your comment.

[jnm2]

Ah, I was answering @DavidArno who suggested that you refactor it into a separate method which would then need a name. It did not escape me that you also need a name for a label, but it the label name is free to be coupled to only make sense in the context of the control flow in the method's implementation whereas refactoring it to a separate method requires it to arbitrarily stand on its own when that was never its purpose.

[iam3yal]

@jnm2 got'cha. 😉

[svick]

@jnm2 Doesn't that change when you use a local function instead of a method? I think that the name of a label and the name of a local function can both be specific to a single method, or even to a scope within a method.

[jnm2]

In theory. That's a good point.

[SirDragonClaw]

Personally I find that coming up with a name for a goto can be very arbitrary and quite counter-intuitive.

Which is why I would prefer to use a 'break integer' syntax, it seems cleaner and more readable to me.

Refactoring tools like resharper should be able to support it fairly easily, I don't think refactoring would be much of an issue.

[jnm2]

Already today a goto label name like exit_foolist_loop is not arbitrary or counter-intuitive.

[iam3yal]

@Danthekilla

Personally I find that coming up with a name for a goto can be very arbitrary and quite counter-intuitive.

Yes, naming is hard, we all know that but just like we come up with names that make sense for other things throughout the code we should do the same here.

goto:

foundValue == valueToCompare ? goto end_of_find_item_loop : continue;

break:

foundValue == valueToCompare ? break find_item_loop : continue;

Readable code should be easy to follow and understand so doing something like break <number> might be easier to understand but not follow as such I don't think it's readable.

p.s. Both Java and JavaScript allows having labels on break and continue so having this in C# would come in handy at times.

[DavidArno]

@jnm2,

(bool found, ValueType value) TryFindValueIn2DStructure(...

I've never really been convinced by the "naming is hard" argument. If you understand what a piece of code does, you can describe it. If you can describe it, you can summarise it. That summary gives you the name. If the summary is long (and likely contains lots of and's or then's), then it's probably doing too much and could do with breaking into smaller parts.

[iam3yal]

@DavidArno "naming is hard" because thinking about it objectively is indeed hard, different people may have different opinions about how something should be named and it's even harder for a name to make sense outside to the few lines of code you or someone else wrote and last but not least code tend to change over time and name things so everything will make sense all of the time, especially the future isn't that simple.

[HaloFour]

I agree with @DavidArno, if you have a need to break from a loop then you have a reason for wanting to do so. It shouldn't be that difficult to summarize that into a quick one or two word label, e.g. FOUND_GROMIT (if you're into the whole upper-case label convention). What's more this is self-describing so other developers (or yourself in six months) should be able to infer what you're doing and why you did it from a glance.

[CyrusNajmabadi]

I am definitely not a fan of introducing a more restrictive lang feature just because someone thinks the existing capability is too permissive. If the feeling is that something is too permissive, then that much better solved with an analyzer vs adding an entire new feature to just provide a subset of existing functionality.

[leoshusar]

Definitely the latter for me.

Interesting, I look at the code and when I see continue inner, I immediately know it means the inner loop should continue.
When I look at the latter, I don't see it when I see the goto, I have to look where that jump label is and why is it at the end of the loop.

I am definitely not a fan of introducing a more restrictive lang feature just because someone thinks the existing capability is too permissive.

But nobody takes away the goto, you can freely continue using it. There are many people who would appreciate this feature, the rest doesn't have to use it. Also considering how many other languages have this.
The same could be said about kinda every feature - why have auto properties? They are too permissive, just create backing field
and getter / setter methods for it, etc.

"Why take this feature away just because someone thinks it's unnecessary for him?"

[CyrusNajmabadi]

?

My argument is the opposite. We already have a permissive feature, so we shouldn't create restrictive features to duplicate them. You can just use an analyzer if you don't like the idea of having non-loop code follow the goto.

But nobody takes away the goto, you can freely continue using it.

Right... that's my point. Since goto works well for this, and is already tehre, and already supports a superset of this functionality with great syntax, i would stick with it. If that superset is not desirable, i would recommend using an analyzer versus just creating a duplicated feature here.

[CyrusNajmabadi]

There are many people who would appreciate this feature,

I get teh ask. I'm just saying i don't think it carries its weight given the ease of writing 'goto' (which still seems great here and very readable to me).

[macias]

What do you mean by 'safer than goto'? What is unsafe about goto?

You write your code basically once, but you maintain it for a long time -- it means you read it more then you write it. So it is crucial the code speaks to you in clear fashion -- goto does not have any special meaning, break/continue has. If someone misplace/misuse the label it is harder to find the bug than with break/continue because the latter have extra information embedded in them -- they are telling the reader the meaning.

Or, shorter version, goto was from the beginning, so why we have break/continue in the first place, if we could live just with goto?

[CyrusNajmabadi]

goto does not have any special meaning

I mean, it means: go to this label :)

If someone misplace/misuse the label

What sort of misuse are you thinking of? If you're worried about placement, that ties into what i was saying above, just add an analyzer to restrict to only teh stylistic uses of goto you're comfortable with.

goto was from the beginning, so why we have break/continue in the first place

Because they don't need labels. If you need labels, at that point, just use 'goto' :)

[leoshusar]

@CyrusNajmabadi Sorry, non-English, confused restrictive with permissive :)

But how would you write an analyzer that knows if you want to goto as a goto or use goto as a break or continue? What if you want to use both in your code? I don't see any easy way to do this.

Also I've written so far 2 super simple analyzers and I don't expect majority of people invest time to learn Roslyn "just for this".

[TahirAhmadov]

Would it be better to use keywords instead of labels?

while(...)
{
  while(...)
  {
    break outer;
    while(...)
    {
      break outer outer;
    }
  }
}

[leoshusar]

@TahirAhmadov what if someone adds another loop? He would have to remember to add another outer to all inner breaks.
In my opinion this is the same as already proposed break 3 which I personally don't like. I see this even more error-prone than goto.

[CyrusNajmabadi]

But how would you write an analyzer that knows if you want to goto as a goto

If you want that, then there's no problem at all as this is the style you like so nothing to worry about. The analyzer is for people that don't like general goto, but are ok using it in a loop.

[CyrusNajmabadi]

Also I've written so far 2 super simple analyzers and I don't expect majority of people invest time to learn Roslyn "just for this".

The majority of people don't need to do this. Someone just needs to write it once and make into a nuget package. If users want this sort of linting, they can choose to use that package. If htey don't care, then there's no issue :)

If users care, but not a single person cares enough to write an analzyer, then it sounds like there isn't much caring after all, in which case i'd def believe there's not a great reason to make this into a lang feature.

[leoshusar]

But still, analyzer would solve an issue for people who don't like goto at all.
My point is, what if someone wants to use both goto and safer loop breaking? What would that analyzer look like?

Also this still wouldn't solve the readability issue, which.. for you is fine, but I would definitely see the loop labels more readable, especially for more complex code than 5 line example.

[CyrusNajmabadi]

and safer loop breaking?

I'm still not sure why it's safer tbh.

What would that analyzer look like?

Maybe say that if a goto is in a loop, it needs to be at the end of the loop. If you want inner gotos and loop gotos, then you have to decide what you want.

[leoshusar]

People, including me, have already written their opinions on why goto is more error-prone.

then you have to decide what you want

You see, analyzer doesn't solve everything this feature would solve. You would have to have either one or the other, not both.

[CyrusNajmabadi]

People, including me, have already written their opinions on why goto is more error-prone.

I don't really get your argument. All you said was: "Interesting, I look at the code and when I see continue inner, I immediately know it means the inner loop should continue."

You can't know that. You have to go figure that out. For example, the code coudl have said:

inner: foreach (...) {
    outer: foreach (...) {

now, the continue inner doesn't mean the inner loop continues, it actually means the outer one continues. You have to go find the starts of the regions to figure this out. This same is true with goto, it's just the ends. And the ends make much more sense to me as that's where execution actually goes to. For example with continue, while the label precedes the loop, that's now wher eyou're jumping to. You're actually jumping to the next iteration. Which to me is clearly indicated by a common point (the loop end). Indeed, that's what it means to hit the endpoint of a loop, that you continue on the next iteration.

Labeling this explicitly and then stating you want to go there makes the most sense to me and actually fits what the language is doing here. It was also so much clearer in the code you had in your example. Indeed with your example code it was much more confusing as now i had to say "oh... i have to be careful if someone goto's this location, as opposed to 'continue'ing to it".

You see, analyzer doesn't solve everything this feature would solve.

If the cases it doesn't solve are so niche, then that's not something that makes me want to add this feature.

If the only reason to have htis feature is so that a subset of users can mix-match gotos and continues/breaks in a loop and have differing semantics for them, then that's so minor that i don't think it's warranted as a language feature.

[leoshusar]

I also said this:

Also with the goto statement someone can put something after the inner: label or before the outer: label, breaking your continue and break intentions.

With the break responseLoop I'm saying "I want to break the responseLoop". And I bet people who are familiar with break and continue would 100% get what that means.

Unlike the goto, you would need to have both breakResponseLoop and continueResponseLoop and ensure they are really in the right place.

Now you can say "so what?" and I agree that this is not a critical feature that the majority of C# programmers will use, but why not have it for the rest? Other languages have it and I'm sure it has it's users. For example Golang is "fairly new" and has both goto and labeled loops. I think they wouldn't add it if they knew nobody would use it.

[SirDragonClaw]

I am definitely not a fan of introducing a more restrictive lang feature just because someone thinks the existing capability is too permissive.

This ignores the fact that GOTO is so permissive that it's use has been banned from the code bases of the last 4 places I have worked at. GOTO and its use is very frowned upon, for many codebases this would be a cleaner replacement, but I understand the downsides too.

That being said I find it amusing you are all still discussing this issue 5 years after I first submitted the proposal 😂

[marcussacana]

That being said I find it amusing you are all still discussing this issue 5 years after I first submitted the proposal joy

Well, I'm here too, and this topic is even 'more or less' the continuation from the old visual studio forum, so it's more than 5 years.
Anyway, to me, feature is feature, the newer C# versions implements some features that I never even really used one time in my life, and maybe I will never use it, but even with that I'm not saying that feature should't be implemented.

We are programmers, not kids, if the goto is good or bad to the code should be the programmer should decide this.
If you or your friends don't need a feature you can just ignore it.

C# is a high level language, so It should aim to simplify the programmer's life where It can, as such I find this feature interesting, and frankly, having to fill the recursive loops relating to the previous loop with booleans and ifs, in my opinion is what makes confusing code, many cases where the code continuation point could be executed where it was necessary to decide where it needs to continue could be solved with this, instead of creating new variables and ifs to check every each loop.

[theunrepentantgeek]

@NigelBess wrote:

Sometimes you inherit a monstrous legacy code base and refactoring into small functions is not an option.

and @leoshusar responded:

But nobody takes away the goto, you can freely continue using it. There are many people who would appreciate this feature, the rest doesn't have to use it.

then @marcussacana wrote:

If you or your friends don't need a feature you can just ignore it.

There's a fallacy here - even a moderate sized project (say, 200kloc) will have multiple developers and maintainers and a lifespan measured in years.

If even one of those developers uses a particular C# feature, then all of those developers needs to be aware of it, else they'll run into code they cannot reason about. The developer who introduced use of the feature may have retired and be living off the grid in an island paradise by the time a new developer runs into the code.

Saying "if people don't like it, they don't have to use it" ignores the reality that most business development involves teams working on a common codebase over several years.

[marcussacana]

That's also because you consider that every programmer works for a company without its own code of conduct,
In that are against whoever is managing the project can just create a specification forbidding the feature.
If the programmer used a forbidden coding style, they must review his own code and check again the code of condut, not the opposite.
Of course, that's supossing the feature is depreciated for the programmers, what we can't say.

[CyrusNajmabadi]

GOTO and its use is very frowned upon

Why is it frowned upon?

[SirDragonClaw]

GOTO and its use is very frowned upon

Why is it frowned upon?

https://www.l3harrisgeospatial.com/Learn/Blogs/Blog-Details/ArtMID/10198/ArticleID/15289/Why-should-GOTO-be-avoided

This was the first result in Google, there are 1000's more. Personally I think it has its uses and banning it from codebases is going to far, but many disagree.

[HaloFour]

GOTO in older languages could be very dangerous. It effectively let you jump anywhere, into and out of functions and other protected areas. The version of GOTO in C# has much stricter guardrails, but that stigma from decades ago remains, likely continuing to be drilled into the minds of students by their professors. You can thank Dijkstra for that.

[CyrusNajmabadi]

This was the first result in Google,

That is 'goto' in idl. I'm asking why it's frowned upon in c#.

[jnm2]

I recommend the use of goto over introducing loops and flags, for the sake of clarity.

goto case is another example of goto being safe and understandable.

[jez9999]

Boy do I with MS would introduce labeled loops. It's literally the one totally valid use-case for goto. And yet they always tell people not to use goto.

Apparently new C# feautres need a 'champion'. Well, I'm not sure why nobody seems to be willing to champion this considering I've seen comments from hundreds of developers who want it.

I'll also just mark here that a related discussion is going on at #5525

[HaloFour]

And yet they always tell people not to use goto.

Because it's not the C# language team telling people to not use goto, they still feel that it's completely appropriate to use that in these cases. The goto keyword in C# is pretty constrained and mitigates most of the issues that led to that paper being written over 50 years ago. Even Djiksta didn't think the advice should be followed dogmatically.