GOTO is it the only way to exit double Foreach?

Question

Possible Duplicate:
breaking/exit nested for in vb.net

Is GOTO the only way to exit from a double ForEach?

For Each city in cities
  For Each person in city.People
    If IsOK(person) GOTO Found_
  Next person
Next city

Found_: 
' ...

The question is for VB.NET, but wondering also for C#...

`Return` is a pretty good way to exit multiple levels of loop also. Sometimes that requires splitting your function so that the loops are in a helper function, but that's usually an improvement to the structure anyway. — Ben Voigt, Dec 28 '12 at 14:45
@serhio: Right now your function does 2 things: It searches for a person, and it does something with the found person. That's a violation of the Single Responsibility Principle, and it splits up nicely into a helper function doing the search (and using `Return`) and a caller that invokes the search helper and then processes the result. — Ben Voigt, Dec 28 '12 at 14:47
if in this C# is similar to Java, a break with a label should work and is nicer than a goto, see here: http://stackoverflow.com/a/886979/559144 have not tested it... — Davide Piras, Dec 28 '12 at 14:47
It's a sign to the OP that he's headed down the wrong path using GOTO this is not Basic this is .NET even though you can use it..it's not recommended — MethodMan, Dec 28 '12 at 14:51
exactly that's why I wrote in my comment what to use either return or break.. — MethodMan, Dec 28 '12 at 14:53
Thanks, as I understand, **the GOTO or a boolean flag is the only way to go out of a double For** — serhio, Dec 28 '12 at 14:58
Depending on your version of VB.Net, Microsoft already provided a solution for this called LINQ. — myermian, Dec 28 '12 at 15:09
This question handles the do's and don'ts of GOTO's rather thoroughly: http://stackoverflow.com/questions/46586/goto-still-considered-harmful I have only ever really needed it in C code to do cleanup (while coding kernel modules and following the guidelines: http://www.kernel.org/doc/Documentation/CodingStyle (Chapter 7) ) — Wouter Simons, Dec 28 '12 at 15:33
the definition of "thing" is quite arbitrary. Say, for find a person you should search in the cities, and then search in the people. So, you can think that each of two FOR are also separate "things", and so on.. — serhio, Dec 28 '12 at 16:20

Charles Bretana · Answer 1 · 2012-12-28T19:19:01.493

9

put it in separate function

  Private Function FindPerson(cities As List(of City)) As Person
     For Each city in cities
        For Each person in city.People
           If IsOK(person) Return person
        Next person
     Next city
     Return Nothing
  End Function

and...

  Private Function ContainsPerson(cities As List(of City)) As Bool  
     For Each city in cities
        For Each person in city.People
           If IsOK(person) Return True
        Next person
     Next city
     Return False
  End Function

EDIT: Fixed VB syntax

edited Dec 28 '12 at 19:19

answered Dec 28 '12 at 14:48

Charles Bretana

143,358
22
150
216

Exactly what my comment was leading toward. +1 – Ben Voigt Dec 28 '12 at 14:50
Yes - read your comment after submitting my post... SRP comment is on target too. – Charles Bretana Dec 28 '12 at 14:52
a smile for VB.NET ) code – serhio Dec 28 '12 at 14:56
1

Thanks, I understand, but as I understand, the GOTO or a boolean flag is the only way to go out of a double for – serhio Dec 28 '12 at 14:57
4

Am I the only one that thinks this should be converted into a LINQ function? – myermian Dec 28 '12 at 14:57
1

m-y, you have my permission to do so... add it to the answer (if you don't have edit rights stick it in comment and I will move it... And thanks to JoelFan for editing answer to correct VB syntax, I appreciate! – Charles Bretana Dec 28 '12 at 15:01
ok... remark : `Cities` should be `List(Of City)` ;) – serhio Dec 28 '12 at 15:14
to comment on the Linq comment... Linq is of course just syntactical sugar, (just like foreach is). The underlying IL code will still look pretty much the same.... but the point of the question ("How to exit from double nested foreach w/0 goto" ) would be obfuscated. – Charles Bretana Mar 19 '13 at 23:04

myermian · Answer 2 · 2012-12-28T15:21:50.567

Why not just use LINQ?

C#:

// Or use SingleOrDefault(...) if there can ever only be one.
var person = Cities.SelectMany(city => city.People)
                   .FirstOrDefault(person => IsOK(person));

if (person != null)
{
    ...
}

VB.Net (my best attempt, I'm not as verbosed in it):

// Or use SingleOrDefault(...) if there can ever only be one.
Dim person = Cities.SelectMany(Function(city) city.People)
                   .FirstOrDefault(Function(person) IsOK(person));

If person Not Nothing Then
    ...
End If

If all you are trying to do is see if there are any IsOK(person), then use the Any(...) extension method instead:

C#:

var isOK = Cities.SelectMany(city => city.People)
                 .Any(person => IsOK(person));

VB.Net (my best attempt, I'm not as verbosed in it):

Dim isOK = Cities.SelectMany(Function(city) city.People)
                 .Any(Function(person) IsOK(person));

It looks like he doesn't need that first person, just knowing if there is a person that `IsOK`, so `Any` would be appropriate instead of `FirstOrDefault`. The primary advantage, other than semantically stating what you mean, would be that it could handle `null` values in the sequence without generating a possible false positive. — Servy, Dec 28 '12 at 15:18
Ah, I thought he was trying to get the value, not just a `true`/`false`. I'll adjust my answer for that. — myermian, Dec 28 '12 at 15:20

score 4 · Accepted Answer · edited May 23 '17 at 12:12

As Heinzi answered in this question:

Unfortunately, there's no exit two levels of for statement, but there are a few workarounds to avoid Goto, which is considered to be bad practice:

Dummy outer block

Do
    For Each item In itemList
        For Each item1 In itemList1
            If item1.Text = "bla bla bla" Then
                Exit Do
            End If
        Next
    Next
Loop While False

or

Try
    For Each item In itemlist
        For Each item1 In itemlist1
            If item1 = "bla bla bla" Then
                Exit Try
            End If
        Next
    Next
Finally
End Try

Separate function: Put the loops inside a separate function, which can be exited with return. This might require you to pass a lot of parameters, though, depending on how many local variables you use inside the loop. An alternative would be to put the block into a multi-line lambda, since this will create a closure over the local variables.

Boolean variable: This might make your code a bit less readable, depending on how many layers of nested loops you have:

Dim done = False

For Each item In itemList
    For Each item1 In itemList1
        If item1.Text = "bla bla bla" Then
            done = True
            Exit For
        End If
    Next
    If done Then Exit For
Next

+1 good answer. But you forget the possibilty to use closures, which makes a "separate" function unnessecary. — igrimpe, Dec 28 '12 at 16:48
Why would you use the Dummy Outer Block technique if it's the same as Goto, but longer and uglier?? Goto is bad practice, but this is even worse. But anyway, the best thing to do is to split into two functions. — Meta-Knight, Dec 28 '12 at 16:53

score 2 · Answer 4 · answered Dec 28 '12 at 14:46

2

You could put a bool in the first loop and set it to false in the inner loop then break. Then in the outer loop break if the bool is false.

answered Dec 28 '12 at 14:46

michaelca

121
3

JoelFan · Answer 5 · 2012-12-28T14:57:01.347

0

You can also throw an exception:

Try
  For Each city in cities
    For Each person in city.People
      If IsOK(person) Throw New FoundException
    Next person
  Next city
Catch ex As FoundException
  DoFoundStuff
End Try

Caveat: My point is just to show that exceptions are an option for exiting multiple nested loops, not that it's appropriate in the particular case of this code example. In particular, exceptions should generally be limited to "exceptional/error" conditions not to "normal" conditions. For example, if you simply changed the "If" to "If Not IsOK ....", the exception may be the right solution.

edited Dec 28 '12 at 14:57

answered Dec 28 '12 at 14:47

JoelFan

37,465
35
132
205

2

That's true, but it's really bad style. – Ben Voigt Dec 28 '12 at 14:48
5

Using exceptions for non-exceptional state and/or regular control flow is a very bad practice. – Honza Brestan Dec 28 '12 at 14:48
I didn't say it was good style but it is another way... why the hate? Also the OP did not limit to just non-exceptional situations... he just said how do you exit a nested loop without goto... sometimes an exception is the right choice for that – JoelFan Dec 28 '12 at 14:49
1

@JoelFan I think acknowledging this is poor style in the answer itself if this is a non-exceptional event, it might help with the downvoters. It is certainly a valid solution and depending on the use case, might be a good solution – psubsee2003 Dec 28 '12 at 14:53
FoundException does not exist ))) so I will not create a new exception only for that – serhio Dec 28 '12 at 14:59
@serhio, that's up to you but it can be created in 2 lines of VB code in the same file... Private Class FoundException: Exception End Class... don't be afraid of adding new classes, they don't cost money ;) – JoelFan Dec 28 '12 at 15:02
ok, this is pure theory, is not good in practice to throw exceptions in cycles like this because of bad performance. – serhio Dec 28 '12 at 15:08

mosca125 · Answer 6 · 2012-12-28T15:01:19.990

0

Add a boolean and set it to true when the person is found and break from the inner loop. then check if found is true. If so then break from the outer loop.

bool found;
For Each city in cities
    For Each person in city.People
        If IsOK(person) 
            found = true
            Exit For
        End If
    Next person
    If (found) 
        Exit For
    End If
 Next city

edited Dec 28 '12 at 15:01

answered Dec 28 '12 at 14:50

mosca125

131
6

I think the VB.NET syntax is `Exit For`, not break. At least that's what VB6 used. – Ben Voigt Dec 28 '12 at 14:51
It's been a long time since i wrote VB.NET! – mosca125 Dec 28 '12 at 14:52

Ralf de Kleine · Answer 7 · 2012-12-28T15:02:53.833

0

Has been ages writing vb.net but the following should work (me thinks)

    Dim found As Boolean
    For Each city In cities
        If found = True Then
            Exit For
        End If
        For Each person In city.People

            If IsOK(person) Then
                found = True
                Exit For
            End If
        Next person
    Next city

edited Dec 28 '12 at 15:02

answered Dec 28 '12 at 14:56

Ralf de Kleine

11,464
5
45
87

Thanks, as I understand, the GOTO or a boolean flag are the only way to go out of a double for – serhio Dec 28 '12 at 15:01
I think so and by my own experience you'd better leave the GOTO alone – Ralf de Kleine Dec 28 '12 at 15:03

score 0 · Answer 8 · answered Dec 28 '12 at 16:47

In addition to 888's answer: An extra function does not necessarily need params to be passed:

Dim a, b, c As Integer
    a = 100
    b = 50
    c = 20

    Dim r = Function()
                For i = 1 To a
                    For j = 1 To b
                        For k = 1 To c
                            If i * j * k = 150 Then Return 1
                        Next
                    Next
                Next
                Return 2
            End Function

    Console.WriteLine(r)

Keyword: closure

GOTO is it the only way to exit double Foreach?

8 Answers8