What's wrong with GoTo

[Max_aka_NOBODY]

I've read many times now that "GoTo is bad, outdated, wrong, hazardous, nasty, and should be avoided at all costs", but just can't see what's so wrong with it. Here's a small bit of code I wrote recently, and I don't see what better way than GoTo is there.

Public Function ReadLIGH(ByVal FileHandle As Integer, ByRef NextRecordType As String) As ES_LIGH
 
With ReadLIGH
    Get FileHandle, , .Size
    Get FileHandle, , .UnknownHeader1
    Get FileHandle, , .Flag
    Get FileHandle, , NextRecordType
ReadNext:
    Select Case NextRecordType
    Case "NAME"
        Get FileHandle, , .NAMESize
        .NAMEValue = Space$(.NAMESize - 1)
        Get FileHandle, , .NAMEValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
        GoTo ReadNext
    Case "MODL"
        Get FileHandle, , .MODLSize
        .MODLValue = Space$(.MODLSize - 1)
        Get FileHandle, , .MODLValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
        GoTo ReadNext
    Case "FNAM"
        Get FileHandle, , .FNAMSize
        .FNAMValue = Space$(.FNAMSize - 1)
        Get FileHandle, , .FNAMValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
        GoTo ReadNext
    Case "ITEX"
        Get FileHandle, , .ITEXSize
        .ITEXValue = Space$(.ITEXSize - 1)
        Get FileHandle, , .ITEXValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
        GoTo ReadNext
    Case "SCRI"
        Get FileHandle, , .SCRISize
        .SCRIValue = Space$(.SCRISize - 1)
        Get FileHandle, , .SCRIValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
        GoTo ReadNext
    Case "SNAM"
        Get FileHandle, , .SNAMSize
        .SNAMValue = Space$(.SNAMSize - 1)
        Get FileHandle, , .SNAMValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
        GoTo ReadNext
    Case "LHDT"
        Get FileHandle, , .LHDTSize
        Get FileHandle, , .LHDTWeight
        Get FileHandle, , .LHDTValue
        Get FileHandle, , .LHDTTime
        Get FileHandle, , .LHDTRadius
        Get FileHandle, , .LHDTRed
        Get FileHandle, , .LHDTGreen
        Get FileHandle, , .LHDTBlue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , .LHDTFlags
        Get FileHandle, , NextRecordType
        GoTo ReadNext
    Case "DELE"
        'Record Deleted. Skip the data.
        Seek FileHandle, Seek(FileHandle) + 8
        Get FileHandle, , NextRecordType
        GoTo ReadNext
    End Select
End With
 
End Function

[bpd]

To show how I might do it, I've removed the GoTos and added the lines shown in red:

Public Function ReadLIGH(ByVal FileHandle As Integer, ByRef NextRecordType As String) As ES_LIGH
 
With ReadLIGH
  Get FileHandle, , .Size
  Get FileHandle, , .UnknownHeader1
  Get FileHandle, , .Flag
  Get FileHandle, , NextRecordType
  
  [COLOR=Red]Do[/COLOR]
    Select Case NextRecordType
    Case "NAME"
        Get FileHandle, , .NAMESize
        .NAMEValue = Space$(.NAMESize - 1)
        Get FileHandle, , .NAMEValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
    Case "MODL"
        Get FileHandle, , .MODLSize
        .MODLValue = Space$(.MODLSize - 1)
        Get FileHandle, , .MODLValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
    Case "FNAM"
        Get FileHandle, , .FNAMSize
        .FNAMValue = Space$(.FNAMSize - 1)
        Get FileHandle, , .FNAMValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
    Case "ITEX"
        Get FileHandle, , .ITEXSize
        .ITEXValue = Space$(.ITEXSize - 1)
        Get FileHandle, , .ITEXValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
    Case "SCRI"
        Get FileHandle, , .SCRISize
        .SCRIValue = Space$(.SCRISize - 1)
        Get FileHandle, , .SCRIValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
    Case "SNAM"
        Get FileHandle, , .SNAMSize
        .SNAMValue = Space$(.SNAMSize - 1)
        Get FileHandle, , .SNAMValue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , NextRecordType
    Case "LHDT"
        Get FileHandle, , .LHDTSize
        Get FileHandle, , .LHDTWeight
        Get FileHandle, , .LHDTValue
        Get FileHandle, , .LHDTTime
        Get FileHandle, , .LHDTRadius
        Get FileHandle, , .LHDTRed
        Get FileHandle, , .LHDTGreen
        Get FileHandle, , .LHDTBlue
        Seek FileHandle, Seek(FileHandle) + 1
        Get FileHandle, , .LHDTFlags
        Get FileHandle, , NextRecordType
    Case "DELE"
        'Record Deleted. Skip the data.
        Seek FileHandle, Seek(FileHandle) + 8
        Get FileHandle, , NextRecordType
    [COLOR=Red]Case Else[/COLOR]
        [COLOR=Red]Exit Do[/COLOR]
    End Select
  [COLOR=Red]Loop[/COLOR]
End With
 
End Function

[Max_aka_NOBODY]

Well, that's just another way. I was asking why any other way is better.

[RhinoBull]

You may want to read this article but you'll need to subscribe.

[WizardLMB]

Goto is a useless piece of code. You can do without it as all.

The case statement does not execute all the code, only the right case.

Use goto statement when you code error handling only. eg

Private Sub HandleError()
On Error GoTo Check:
 
'Code with no errors here
 
 
 
Check:
Select Case Err.Number
Case Is = 12323333
'code here
 
Case Is = -345599
'Code here
 
Case Else
'Code here for all other erros
MsgBox Err.Description & Err.Number, vbOkOnly, "Error"
 
End Select
 
End Sub

I hope this will help you.

Wizard
SA

[RhinoBull]

Goto is a useless piece of code. ...

I would totally disagree.

... You can do without it as all. ...

He knows that but it's not what he asked.

[Max_aka_NOBODY]

The registration at that site doesn't seem to be too fast... Waiting for the confirmation EMail for 10 minutes now - not that much, but still unnatural. Anyway, indeed I do understand that I can perform most tasks without it, but the exact problems that could be produced from using GoTo are still not showing anywhere on the horizon...

[moeur]

This is a little bit of a "religious" topic. In the early days of BASIC, Goto was the only way you could do certain things, but it often led to the tangled code people refer to as "Spaghetti" code. You know a plate of tangles noodles, finding the end of one noodle means you have to trace through a bunch of other ones.

With the advent of modern BASIC, there is much less need for Goto so if you avoid using it you can help insure your code is easier to read and debug. That being said, there is no reason not to use goto if it makes your code simpler as in the following example.

While Not Finished
    For i = 1 To 100
        If SomethingHappened(i) Then GoTo ExitAllLoops
        DoOtherStuff
    Next i
    Finished = CheckStatus
 Wend
 MaybeDoOtherStuff
ExitAllLoops:

There are ways to do the same thing without Goto, but not as simply.
IMHO, feel free to use Goto, just don't get carried away with it.
...

[pnish]

The Goto debate is one of those issues where everybody has an opinion. I would think the greater majority would say Goto is BAD, but then you ask them 'why?', they say, 'um err, well I read somewhere that you shouldn't use it'.

I also think the use of Goto (other than On Error Goto), is unnecessary. It can make code difficult to read (and therefore debug) by interrupting the logical top, down flow of a procedure.

Most VB apps, being event-driven, can be challenging to debug due to the fact that you can never really predict what the user is going to do. The use of Goto only makes it worse.

As I said, everyone's got an opinion. That was mine. Cheers.

[WizardLMB]

Sorry, my mistake.

Thanks

Wizard

[RhinoBull]

I am no expert on compilers but I think that GoTo is somewhat difficult to swallow so optimization could be in geopardy.

[Wokawidget]

Do While Not Finished
    For i = 1 To 100
        If SomethingHappened(i) Then Do
        DoOtherStuff
    Next i
    Finished = CheckStatus
Loop
If Finished Then
    MaybeDoOtherStuff
End If

Why use something that has the potential to breed extremely complicated code, which is hard to debug, when there are alternative methods, which provide much easier and neater code?

Gotos and Gosubs were used because there were no such things as subs or functions.
So in Basic, you would have had (Amstrad CPC 6218):

10 Print "Woof"
20 Gosub 80
30 Print "Growl"
40 Gosub 80
50 Print "Moose"
60 Gosub 80
70 End
80 For x = 1 To 10
90 Print x + 2
100 Next
110 Return

Only line I am not sure about is 70. Hmmmm...anyways. Gosubs are required here. It creates a kind of function.
In VB6 this would be like doing:

Private Sub Form_Load()
   Debug.Print "Woof"
   DoStuff
   Debug.Print "Growl"
   DoStuff
   Debug.Print "Moose"
   DoStuff
   Unload Me
End Sub

Private Sub DoStuff()
Dim x As Long
   For x = 1 to 10
      Debug.Print CStr(x+2)
   Next x
End Sub

One question. You do use Option Explicit at the top of your code right?

Woka

[ccoder]

there is no reason not to use goto if it makes your code simpler as in the following example.

While Not Finished
    For i = 1 To 100
        If SomethingHappened(i) Then GoTo ExitAllLoops
        DoOtherStuff
    Next i
    Finished = CheckStatus
 Wend
 MaybeDoOtherStuff
ExitAllLoops:

There are ways to do the same thing without Goto, but not as simply.
IMHO, feel free to use Goto, just don't get carried away with it.
...

Excellent example! You may be happy to know that there was an article in Communications of the ACM about 15 years ago that basically said the same thing.

[techgnome]

That being said, there is no reason not to use goto if it makes your code simpler as in the following example.

While Not Finished
    For i = 1 To 100
        If SomethingHappened(i) Then GoTo ExitAllLoops
        DoOtherStuff
    Next i
    Finished = CheckStatus
 Wend
 MaybeDoOtherStuff
ExitAllLoops:

Whooo boy.... that's dangerous.....
Consider this:

Do While Driving
  For intX = 1 to 100
    If DestinationReached Then Goto DestinationAchieved
    KeepDriving
  NExt
Loop
objCar.Stop
objCar.Ingition = enmOff

DestinationAchieved:
Exit Car

What is *wrong* with that? Can *anyone* (who advocates the use of goto) see the inherent danger (or not?) in that piece of code?

Tg

[moeur]

Do While Not Finished
    For i = 1 To 100
        If SomethingHappened(i) Then Do
        DoOtherStuff
    Next i
    Finished = CheckStatus
Loop
If Finished Then
    MaybeDoOtherStuff
End If

I assume you meant Exit Do. In any case this technique adds an extra statement which can easily get out of had if the nesting gets much deeper.

...

[Wokawidget]

I really hope this user doesn't mind me using their app as an example.
But anyways download the zip file in Post 16 of the following thread:

http://www.vbforums.com/showthread.php?t=321688

It's worth having a look at the whole code. It's quite amazing. That user must be a genius to have written that code given all the problems with it.

Private Sub POP_DataArrival(Index As Integer, ByVal bytesTotal As Long)
POP(Index).GetData inp$
WaitF(Index) = WaitF(Index) + inp$
If Right$(inp$, 2) = vbCrLf Then inp$ = Left$(WaitF(Index), Len(WaitF(Index)) - 2): GoTo 6 Else Exit Sub

6 'MsgBox inp$
xp = InStr(1, inp$, " ")
If xp = 0 Then hdr$ = inp$: inp$ = "": GoTo 4

hdr$ = Left$(inp$, xp - 1)
inp$ = Mid$(inp$, xp + 1)
4 Select Case LCase$(hdr$) 'Parse request
    Case "user"
        Conn(Index) = inp$ 'Wait for password
        Send Index, "+OK User name received, please use PASS command to log in." + vbCrLf
        
    Case "apop"

That is a snippet of the code from the DataArrival event of a winsock control.
Notice how they're using "line numbers" as their goto labels. Plus, they are not even in order. Does 6 come before 2, and 1 afer 3...Errrr...Now tracing the code is getting REALLY hard.
This is a perfect example of how not to write code.
Oh, and they don't use Option Explicit either...Now you're really knackered trying to find bugs.

If you want a reason not to use Gotos then that code is it.

Woka

[szlamany]

This is always a religious war - but I can't resist.

The concept of moving the "program pointer" to a new location is a foundation of programming.

Assembler could not do without this - it's paramount.

The belief that GOTO is responsible for spaghetti code is flat out wrong - bad programmers create spaghetti code and use GOTO to make it worse.

EXIT SUB, EXIT FUNCTION, RESUME - these are all exactly the same as GOTO - absolutely no difference whatsoever.

We develop some "loop" extensive code here - report writer engines, student schedule algorithms - we appreciate the ability to use GOSUB - a quick way to jump to "code logic" and then return to a previous location without requiring the framing of memory that a function/sub call would do. When you are looping through 100,000 rows to produce report output - this makes a difference (in our opinion). VB.Net removed GOSUB as an option (we hear) so we will have to re-code the handful of spots we used it when we migrate (or use a lower-level language - assembler - to process this logic).

At any rate, it's always nicer to use SELECT/CASE and BOOLEAN values and LOOPING for ease of future modifications - so GOTO really doesn't find a place when those other constructs are used instead.

[dglienna]

GOTO is from the days of Quick and Dirty programming. Patch it to make it work. Hence, the use in speghetti code. If it made it work, then cool.

Not for well-though-out programs, and it's rare that you need it to make code work, unless you are adding it to a poorly designed program to begin with.


GOTO is *usually* bad, and generally unnecessary, but not un-needed.

[dw85745]

I'm in complete agreement with szlamany. In fact when most code is disassembled, or written in assembly, alot of statements are "JUMP" which is a GoTo.
Disassemble one of your own programs to get a good feel.

Like any programming instruction it has its purpose. I can remember
when this debate stated (about 20-25 years ago). Guess what - GoTo
is still here. In fact, sometimes you can't get to where you want without
it.

David

[pnish]

In fact when most code is disassembled, or written in assembly, alot of statements are "JUMP" which is a GoTo.

Spot on.
And we all know how easy it is to debug a 500 line program written in assembler.

[dglienna]

GoTo's disappeared with "Structured Programming". They were commonly used in COBOL programs, despite other methods that were better, hence more 'structured'.

[bpd]

It's not that GoTo is "bad", it's just that there are so often better alternatives. IMHO, as a rule, structured programming constructs are superior to the use of GoTo. But there will always be exceptional occasions.

when most code is disassembled, or written in assembly, alot of statements are "JUMP" which is a GoTo.

For straight assembly, GoTo is virtually required. That's because of a general lack of better constructs in assembly, not because of any virtue held by GoTo.

I can remember when this debate stated (about 20-25 years ago).

1968. Go To Statement Considered Harmful by Edsger W. Dijkstra

I like what Steve McConnell has to say about this topic in his book, Code Complete 2. On page 399, Steve writes:

Good programming doesn't mean eliminating gotos. Methodical decomposition, refinement, and selection of control structures automatically lead to goto-free programs in most cases. Achieving goto-less code is not the aim but the outcome, and putting the focus on avoiding gotos isn't helpful.

[liverpool_rule]

Call?

[i00]

ok 1st of all i dont believe that goto is a bad thing:

- techgnome said that this could be a prob in response to ccoder's code:

Do While Driving
  For intX = 1 to 100
    If DestinationReached Then Goto DestinationAchieved
    KeepDriving
  NExt
Loop
objCar.Stop
objCar.Ingition = enmOff

DestinationAchieved:
Exit Car

but ccoder didn't say the content his code was to be used in - consider this:

While Not AtDest
    For i = 1 To RestPoint
        If SomethingHappened(i) Then GoTo EmergencyProceadure
        RestPoint=GetNextRestPointOnTrip()
    Next i
    AtDest= CheckDest()
 Wend
 HaveLunchAtDest()
exit sub
EmergencyProceadure:

also whats wrong with doing this?

Do While Driving
  For intX = 1 to 100
    If DestinationReached Then Goto DestinationAchieved
    KeepDriving
  NExt
Loop

DestinationAchieved:
objCar.Stop
objCar.Ingition = enmOff
Exit Car

so whats so "dangerous" now?

also Wokawidget said that "If you want a reason not to use Gotos then that code is it." - What a stupid thing 2 say - if you want you can find bad examples of anything - that doesn't nessessarly make doing those things bad. For example i could use Wokawidget's logic and say "Person X is a bad driver - therefore driving is bad."
I agree that in that case gotos were used in a bad way - but still goto's can make benifit your code if used correctly. ccoder provided an excelent example.

All of that said; I also think that goto should not be used solely as a replacement for do's or for's but, if used correctly, be a compliment to them.

[dglienna]

I've seen GOTO's used in Programming Tests, using a completely made-up language. Print, Get, and GOTO! You have to figure out the value of X after it gets calculated in a midst of speghetti code. I always managed to figure out the correct answers, so as a learning tool, seeing them is good. Using them is not, usually.

[Wokawidget]

also Wokawidget said that "If you want a reason not to use Gotos then that code is it." - What a stupid thing 2 say - if you want you can find bad examples of anything - that doesn't nessessarly make doing those things bad. For example i could use Wokawidget's logic and say "Person X is a bad driver - therefore driving is bad."

True true. But I would use "Person X is a bad driver when driving drunk- alcohol is a bad thing."

I see your point.
I was trying to show how, if you don't write structured code, and you don't control Gotos, then your code can end up pretty messed up if you are not very carefull.

You say potato I say potato...hmmmm doesn't really work when you write then sentence...Hahaha

Woka

[Max_aka_NOBODY]

All that is good, but I still am with GoTo all the way.

Here, I rebuild my whole 2000-lines read module(of many functions on the same framework as the one I posted earlier) with Do...Loop, and upon reading a file got quite a HUGE performance hit. Assuming it was a coincidence, I did a benchmark, and unless I messed something up in the whole benchmarking module, my previous GoTo method is faster.

The Do... Loop method below gives around 1,460,000 duration per second:

Private Sub Performance_Test()
 
Dim var As Long
Milliseconds = GetTickCount
SetThreadPriority GetCurrentThread, THREAD_PRIORITY_HIGHEST
SetPriorityClass GetCurrentProcess, HIGH_PRIORITY_CLASS
Do
    If GetQueueStatus(QS_HOTKEY Or QS_KEY Or QS_MOUSEBUTTON Or QS_PAINT) Then DoEvents
    Frame_Count = Frame_Count + 1
    If GetTickCount - Milliseconds >= Milliseconds_Per_Second Then
        Get_Frames_Per_Second = Frame_Count
        Caption = Get_Frames_Per_Second & " durations per second"
        Frame_Count = 0
        Milliseconds = GetTickCount
    End If
    var = 0
    Select Case var
    Case Is < 10000
        var = var + 1
    Case Is < 20000
        var = var + 1
    End Select
Loop
 
End Sub

While my GoTo method gives around 1,520,000 durations per second:

Private Sub Performance_Test()
 
Dim var As Long
Milliseconds = GetTickCount
SetThreadPriority GetCurrentThread, THREAD_PRIORITY_HIGHEST
SetPriorityClass GetCurrentProcess, HIGH_PRIORITY_CLASS
Label_01:
    If GetQueueStatus(QS_HOTKEY Or QS_KEY Or QS_MOUSEBUTTON Or QS_PAINT) Then DoEvents
    Frame_Count = Frame_Count + 1
    If GetTickCount - Milliseconds >= Milliseconds_Per_Second Then
        Get_Frames_Per_Second = Frame_Count
        Caption = Get_Frames_Per_Second & " durations per second"
        Frame_Count = 0
        Milliseconds = GetTickCount
    End If
    var = 0
    Select Case var
    Case Is < 10000
        var = var + 1
        GoTo Label_01
    Case Is < 20000
        var = var + 1
        GoTo Label_01
    End Select
 
End Sub

As you see, even in this micro-loop it gives quite a difference. And since my functions are intended to read files up to 100MB and break them byte-by-byte, performance is more than important.

[szlamany]

It's not that GoTo is "bad", it's just that there are so often better alternatives.

Good programming doesn't mean eliminating gotos. Methodical decomposition, refinement, and selection of control structures automatically lead to goto-free programs in most cases. Achieving goto-less code is not the aim but the outcome, and putting the focus on avoiding gotos isn't helpful.

BPD has summed it all up in this statement. This is the whole point.

GOTO really is a low-level language construct - it probably makes up 25% of all "assembled" code. Every SELECT/CASE and IF/THEN/ELSE ends up being a series of ASSEMBLER JUMP statements.

Using this low-level construct in a 3rd gen language like VB instead of using IF/THEN and SELECT/CASE means you are missing the whole point of these other constructs. A SELECT/CASE is designed to be easily added to - a new case fits in 6 months later with ease. I remember before SELECT/CASE migrated into the BASIC syntax. We were forced to use IF/THEN/ELSE - many indents - 6 months later you wanted to add to this - forget it.

The point that spaghetti code and GOTO are really based on the fact that when revisiting code for modification if you are not fully aware of all the GOTO's above your modification point you might be adding code that will never be reached. That is a logic flaw in the program construction that has nothing to do with GOTO, but is manifested by the GOTO. The logic was failed to begin with.

After 25 years of developing large apps that are constantly modified and enhanced we have fallen upon setting BOOLEAN variables and using them in IF/THEN statements - this allows us to add new IF's later, based on BOOLEANS above.

Good logic - properly constructed - can only be done one way. In the end there is only one single way to construct a routine. You might be presented with 4 variations - but when it comes down to it - there is really one that would rise to the top if fully scrutinized. The goal of good logic is that it's clear and easy to follow. That leads to programs that can be maintained easily.

Since my programming shop makes most of it's dollars off of maintenance contracts this is extremely important to us.

[RhinoBull]

...You say potato I say potato...

... forever young Louis Armstrong ... I love that song - it's simple but very philosophical ...