dotnet / csharplang

The official repo for the design of the C# programming language
11.59k stars 1.03k forks source link

Discussion: C# Break nested loop #869

Closed jcouv closed 2 years ago

jcouv commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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

iam3yal commented 7 years ago

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 commented 7 years ago

I'd prefer to see something like Java Branching Statements, as once pointed out by @HaloFour at https://github.com/dotnet/csharplang/issues/176#issuecomment-310874877

jnm2 commented 7 years ago

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 commented 7 years ago

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

benaadams commented 7 years ago

Inline returns slow down loops until .NET Core 2.1 https://github.com/dotnet/coreclr/pull/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 commented 7 years ago

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 commented 7 years ago

@bondsbw Not to mention copy/paste.

JarrydSemmens commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@eyalsk Exactly my point. 😄

jnm2 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

@jnm2 got'cha. 😉

svick commented 7 years ago

@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 commented 7 years ago

In theory. That's a good point.

SirDragonClaw commented 7 years ago

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 commented 7 years ago

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

iam3yal commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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.

Joe4evr commented 7 years ago

Also related.

tpetrina commented 7 years ago

Maybe immediately invoked functions with return could help with the naming problem...

iam3yal commented 7 years ago

Just to clarify, my comment earlier was in regard to "There are only two hard things in Computer Science: cache invalidation and naming things." and really the way I think about the word hard in this context is you need to think hard about naming stuff but yeah probably, if you can explain a piece of code as @DavidArno said then one can come up with a reasonable name, it might not satisfy everyone but at least it would be reasonable.

gulshan commented 7 years ago

Can I put the label and loop from @HaloFour 's example in the same line?

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;
    }
}
iam3yal commented 7 years ago

@gulshan In JavaScript it's allowed, I'm not sure about Java though, haven't used this feature there.

HaloFour commented 7 years ago

@gulshan Yes

orthoxerox commented 6 years ago

The VB LDT has positively reviewed a similar feature where the loop is referenced not by its level or its label, but by its loop variable. This won't work for C#, as only foreach is guaranteed to have a loop variable.

gafter commented 6 years ago

foreach is no longer guaranteed to have a loop variable.

  1. foreach (var _ in e) ...
  2. foreach (var (x, y) in e) ...
ghost commented 6 years ago

@jcouv @DavidArno @jnm2 @eyalsk @Logerfo @HaloFour @orthoxerox @gulshan @Joe4evr

In #2024 , I have also suggested:

break for;
continue foreach;
break while;
break all; 
break parent;
continue parent; 

If they are used in nested loops of the same type, I suggest to use the loop counter as an identifier:

for (var i=0; …………)
{
     for (var j=0; …………)
          break i;
}

And Maybe it is better to label nested loops if needed, and use the label with break and continue:

Loop1: while{

               while {
                    break Loop1;
                    continue Loop1;
                }
            } 
juliusfriedman commented 5 years ago

Yes... make a LoopState or LoopContext and refer to that upon exit.

It doesn't have to be complex but the compiler should not be doing things like this because the question becomes when where and how not Why and it's why that always bites.

Ntl.

Break from within another switch or while now has different semantics especially if performed at the end of a loop...

I vote no.

bmuessig commented 5 years ago

It's a dearly lacking feature. I vote yes.

jez9999 commented 5 years ago

I fully support implementing labeled loops. They would be relatively easy to implement (basically syntactic sugar) and look a bit less ugly than using goto:

outerloop:
while (...) {
    while (...) {
        if (...) { continue outerloop; }
        break outerloop;
    }
}

Rather than:

while (...) {
    while (...) {
        if (...) { goto continue_outerloop; }
        goto break_outerloop;
    }

    continue_outerloop: {}
}
break_outerloop: {}
troien commented 5 years ago

I like this idea as I really want this to be simpler then it is now, but personally I would prefer a different syntax.

break break; break break break; break continue; // this would break out of current loop/switch and continue in 'parent' loop

Where you can basically add as many 'break' as you need, but there can only be one continue and continue has to be the last. (as it wouldn't make sense to continue and then immediatly break)

while(...)
{
    while(...)
   {
      if (...) break break;
   }
}
while(...)
{
    if(...)
        break break; // should give compiler error CS0139
}

The advantage of this would be that the break statements are still linked to a loop at compile time, meaning the compiler can treat them as if they were placed in the enclosing loop (or switch), show compiler errors and when putting your cursor on a break statement, it should still be able to highlight the correct loop just like it does now.

marcussacana commented 4 years ago

I discussed about this feature as well in the vs dev community 2 years ago. Nothing implemented yet but should be good see implemented one day, To me break 2; or break label_name;, continue label_name; looks good.

merarischroeder commented 3 years ago

I noticed today that Rust has this kind of thing: break 'outer; - it's nice! see https://doc.rust-lang.org/rust-by-example/flow_control/loop/nested.html

I came looking to see if C# has this yet. I'm hoping it comes along one day.

So Java and Rust have this kind of language feature. Are there any other languages with this apart from Java and Rust?

(Rust uses a dashed syntax for loop labels, because they also support Loop Expressions - https://doc.rust-lang.org/reference/expressions/loop-expr.html#continue-expressions)

TahirAhmadov commented 2 years ago

I, too, would support labeled break statements similar to Java and Rust. Existing label syntax should be used to "name" while, for, foreach, switch, do constructs; these labels can gotoed, breaked, and continued.

leoshusar commented 2 years ago

+1 for labels for me.

For example Dart has also labels like these:

outer: for (final a in [1, 2, 3]) {
  inner: for (final b in ['A', 'B', 'C']) {
    if (b == 'C')
      continue inner;
    else if (a == 3)
      break outer;
    print ('$a $b');
  }
}

and Kotlin even allows to return from lambda functions like forEach:

listOf(1, 2, 3, 4, 5, 6).forEach loop@{
    if (it % 2 == 0) return@loop
    print(it)
}
// prints 135

but here you can also omit your label and use implicit label, like return@forEach

TheBlackPlague commented 2 years ago

From what I can infer, most people agree that some sort of in-context multi-level breaking syntax is needed. In which case, why can't we change the label on this to Proposal? @jcouv / @mattwar

DavidArno commented 2 years ago

@TheBlackPlague,

Since the OP for this discussion has 39 upvotes and 30 downvotes, I'd suggest that "most people agree that some sort of in-context multi-level breaking syntax is needed" is a strange conclusion to reach here. 😄

jez9999 commented 2 years ago

I don't know why anyone would downvote this as a general idea. I mean, what skin is it off anyone's nose if it gets implemented and they don't like it? Just don't use it. Also note that the OP proposes using break 2 rather than break label, and many people might be downvoting the 2 part (myself included) while thinking the label-style branching statement is a very good idea.

Sadly, owing to the indifference and irrational hostility to the proposal in the C# community that seems to exist I've given up hope of it ever being implemented, which sucks, because I'm forever going to be using labels pointing to empty blocks along with gotos to achieve the same result.

CyrusNajmabadi commented 2 years ago

I don't know why anyone would downvote this as a general idea.

People are likely not down voting the general idea, but rather the specific solution outlined in the OP.

CyrusNajmabadi commented 2 years ago

Sadly, owing to the indifference and irrational hostility

Painting those who disagree with you as irrational is a violation of the .net code if conduct. As we say there:

Examples of behavior that contributes to a positive environment for our community include:

Demonstrating empathy and kindness toward other people Being respectful of differing opinions, viewpoints, and experiences Giving and gracefully accepting constructive feedback

People have explained constructively what they do not like. It's on those who wish to see this proposal to succeed to see if there's a way to navigate this and produce a better proposal the community will respond more favorably to

DavidArno commented 2 years ago

I don't know why anyone would downvote this as a general idea

I can only speak for myself, but I actively downvote all proposals around labelled breaks as I strongly dislike the general idea.

The reason I dislike it is because it removes the pain involved in handling deeply nested conditionals. There's reams of information on the web around why deep nesting is a bad idea with plenty of advice on refactoring such code. With that in mind, C# current approach of requiring the use of goto to break out of deep nesting is an example of "pit of success" functionality. You have to work hard at writing code that climbs out of the success pit. Adding labelled breaks makes it easier to write deeply nested code, which goes against that pit of success principle and so I voice my opposition to any such change.

marcussacana commented 2 years ago

If I want to work hard for simple tasks I would be using assembly