csharplang
csharplang copied to clipboard
dotnet
Discussion: C# Break nested loop
@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?
@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.
Sometimes it's a loop and a switch and coming up with a name for it is super arbitrary and counter-intuitive.
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:
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
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.
@Logerfo, Indeed, it makes things clear than having something like break <number>.
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.
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.
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.
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.
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?
@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.
@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.
@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.
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.
@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.
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.
Already today a goto label name like exit_foolist_loop is not arbitrary or counter-intuitive.
@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.
@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.
@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.
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.
Maybe immediately invoked functions with return could help with the naming problem...
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.
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;
}
}
