So...why not?

[Bill-E-BoB]

People always tell me never to use Goto statements in my code, but nobody has ever really given me a valid reason why not. Its always sort of been a "because I said so" argument. In fact one guy I know told me that his professor in college said if any code contained a goto, he would fail the student on the spot.

So, whats the real reason this is such a no-no? I've written stuff that programmers in this company have been trying to accomplish for years...and I do most of it with the right combination of If/Then statements & Goto's. When working with spreadsheets, a goto statement lets me perform the same chunk of code over & over on every line of the spreadsheet whether it be 5 or 5000 rows, and as long as the statement ends with (A = A + 1, if wkst.cells(A,1) <> "" then Goto START:,) it works like a charm...I never have to worry about a predefined number of variables, it adapts itself to whatever source data I'm working with, and the code runs as fast as whatever system I'm working in...if all the functions it performs take place in an excel spreadsheet, it can complete a full spreadsheet of 1000+ rows in under a second and has never caused an error for me.

So, what disadvantages are there to a goto statement really? Sorry if its a dumb question, but the only way I can improve my coding is by asking questions...even the dumb ones.

[RhinoBull]

So...why not?.

One reason is because it creates "spaghetti code" wich is very difficult to maintain and debug too.

[Comintern]

I just had to debug and re-write portions of a program that made extensive use of Gotos, and it is a real nightmare. I literally took me 3 weeks to figure out what all of the functions were doing and a week to re-write the entire thing. The main reason is that you can't just glance at the code and get a good idea of what the program flow is.

That said, there are really easy ways to avoid Goto use. The example you give is functionally identical to this:

'Using Goto
Start:
    '...
    A = A + 1
    If wkst.Cells(A, 1).Value <> vbNullString Then
        GoTo Start
    End If
 
'Not using Goto
    Do
        '...
        A = A + 1
    Loop While wkst.Cells(A, 1).Value <> vbNullString

If you have no idea what the code is supposed to be doing (i.e., it isn't your code or you haven't looked at it in months), the second example is a lot clearer.

[Joacim Andersson]

Instead of having Goto START you could have made a function call or perhaps put it in a loop. The reason Goto statements are considered bad is because the code may be hard to follow. People love to read code that is correctly indented, so you press Tab after an If statement for example and unindent it where you have the End If part. Now without reading the code inside the If statement I can look at the code and immediatly see where it continues if the If statement would be False. But if you instead use a Goto statement I have to start searching for that.

This cause something often referred to as spaghetti coding since you can have nested goto statements that takes you to, at first glance, an illogical point in your code. Using correct stuctures and blocks in your code takes away the need for Goto statements altogether. You can always write the code so that a Goto is not necassary (with the possible exception of On Error Goto statements in classic VB, since it lacks structured error handling).

You should never just write code that works. You should also write code that is maintainable. Writing code that works but isn't easily maintained will at the end be very costly for the company that is stuck with the source.

[bpd]

An extract taken from http://www.ascotti.org/programming/cc/flow.htm:

Everyone has heard that goto is evil and has to be avoided at all costs. Well... that's true, but why? In a few words, the answer is that goto takes away from our ability to understand how a program works.

When we write a program, we face the rather difficult task of describing the behavior of a dynamic process, a program in execution, with a completely static notation: the source code. This is possible because we use the static representation to visualize in our mind the dynamic behavior of the program well before it is actually executed. It is extremely important that the static notation allows us to describe the desired behavior in the most direct and natural way, because each unnecessary step makes it more difficult for us to understand the dynamic process and anticipate the result.

Structured programming is so good because it provides a notation that is very linear and easy to understand. You can read a piece of code and at runtime the program will do exactly what you read, in the order you read it. And although loops and function calls do require some more mental resources to keep track of them, that is easily within the grasp of everyone.

This is not so when goto is used. The linearity is lost and each time you encounter a label that can be the target of a jump you are left to wonder: what is the immediate past of the program, which path has it followed to arrive here? And not too much later: what the darn thing is supposed to do?

[Bill-E-BoB]

That makes sense. I've done the Do & LoopWhile thing before...and for some reason just never thought of using it for these same purposes.

Some of the codes I've written are definately worthy of a little marinara sauce by these definitions...though to be honest, the labels have always helped me understand what its doing more clearly...I never nest a goto statement within another...because I have before, and it turns messy REAL fast. I'll either block a section out by labeling it & ending it with the A+1 & Goto thing, or use 'em for error resolution because quick & easy. For example, one code I wrote compiles the daily order assignments for my organization. It performs 10 different steps to do this and I wrote each step separately & used a label at the beginning of the section. It just repeats the section until (A,1)="" and then moves on to the next section. I guess the same thing could be accomplished with the loop function suggested by Comintern and look more clean in the event that another programmer has to dig into my code.

The only thing is, with a loop you have to put a comment in there explaining what you're doing...with goto statements the comment is already sort of done for you, I guess thats why they seem so easy to follow to me. A section of a report starts out with a label [ASSIGN_ORDERS:] and ends with [If (a,1) <>"" then Goto ASSIGN_ORDERS:] that tells me exactly what its doing during that section of code. With a loop, I would have to start it out with a comment that says 'This section will assign orders and loop until all orders are assigned. Not a big deal obviously, but as long as you don't try to cross up your goto statements it seems like a loop would require more work.

[Joacim Andersson]

That's the very first time I ever heard somebody say that using labels and goto statements creates self-documented code. I often hear the opposite though.
Why not create a Sub called AssignOrders and put the loop inside that procedure. That would also make it self-documented.

[penagate]

Great thread

Is there something/somebody requiring you to comment all loops? Because if you use simple and relevant variable names, I would say most loops are more self-explanatory than Gotos. Possibly you are just used to the Goto way and I am used to loops... But with both pre-test and post-test loops as well as For I really don't see how Goto makes things any clearer.

[leinad31]

Structured programming forces you to step back every now and then and get a feel for the bigger picture on how the short pieces of algorithm best fit together... rather than just limiting your view to where you are and where your gonna jump to next...

My impression is that your stuck in no man's land and not getting the benefits of both sides. Your not a true spaghetti coder but not a true structured prorgammer.

You impose structure on your code (eg. avoiding crossing of Gotos, etc) but didn't exert more effort to make it more structured without the Gotos. You don't use procedure calls (and get the benefit of debugging with Call Stack Viewer) and instead create long streams of code... which you make "modular" by segragating with labels. Your using assembly language coding practices on an OOP language.

And then there's the problem of managing your references... by jumping around you may leave numerous object references which can cause memory leaks, or acciedntally reset the data of object in an intermediate jump, and other unforseen problems... it will become more and more difficult to debug in time.

Your non conformance to was is expected as appropriate simply because it still works will lead to confusion and difficulties for those who will be reading/updating your code in the future.

[dee-u]

The only argument I can find in favor of Goto is job security, since you may only be the one who could understand the code then probably the company may keep you longer...

[Hack]

Moved to General Developer

You should never just write code that works. You should also write code that is maintainable. Writing code that works but isn't easily maintained will at the end be very costly for the company that is stuck with the source.

This statement should be enscribed over the doorway of every programming IT area!

[szlamany]

bpd's quote really said it clearly.

It's really not the GOTO that is evil - it's the destination label that is evil.

That label can be arrived at from several GOTO statements making the logic-flow unclear and broken.

The pitfall in modifying a program with lots of GOTO/label statements is that unless you fully and clearly review the entire algorithm you cannot add code without the possibility of breaking the existing logic flow. Since that logic flow is not singular - it's even more likely.

Spaghetti code defines this problem visually - several paths of logic all messed up in a pile.

EXIT SUB is a GOTO to leave the function. That is acceptable - that is because it leaves the function - there is no destination label that when you come upon it you wonder "how many places come to here?". [edit] Although too many EXIT SUB's creates a nightmare to work with as well!

As HACK pointed out - the Joacim quote is priceless... Someone has to visit your code in the future and maintain or enhance it - that person deserves a properly structured logic flow.

[kfcSmitty]

I think the best way for you to see the problems with GoTo's is to simply try to write the logic down in a flowchart.

you'll see lines flying across your paper at every jump. Gives me a headache :/

[RhinoBull]

The only argument I can find in favor of Goto is job security, since you may only be the one who could understand the code then probably the company may keep you longer...

I'd rather let you go and have someone re-write that darn thing... Sorry.

[Hack]

I'd rather let you go and have someone re-write that darn thing... Sorry.

Nothing to be sorry about RB...I would too.

[Shaggy Hiker]

By the way, it should be noted that there is little performance cost to a GOTO. It's not like it is inferior in speed, just that it makes maintenance harder. Having said that, I believe that use of GOTO in the fashion you describe would utterly prevent the rare circumstance where you can evaporate an If statement, which does improve performance. However, evaporating If statements only works with certain numerical situations and can be so mind bending (because you actually execute both branches of the If) that doing so is not a good idea unless you need the absolutely fastest code.

[techgnome]

In of itself, GOTO is not inherently bad or evil.... just like any thing else in programming, it's the use of it that makes it bad. And just like most things, there are certain tools for certain situations. The only time I ever type GOTO is in On Error.... primarily because I have no other use for the GOTO. It's an old cary over from the non-procedural days of BASIC, and like the appendix, it hangs around with no real purpose, other than to cause pain and grief once in a while.

-tg

[Hack]

...and like the appendix, it hangs around with no real purpose, other than to cause pain and grief once in a while.

If there is a more perfect analogy for GOTO, I have never heard it.

[Al42]

The only time I ever type GOTO is in On Error.... primarily because I have no other use for the GOTO.

If the error routine has the same cleanup code as the main sub, it's not evil to end the error trap with a GOTO the cleanup in the main routine. Something like

'do stuff
 
Cleanup:
  If rsADO.State = adStateOpen Then rsADO.Close
  Set rsADO = Nothing
  If conADO.State = adStateOpen Then conADO.Close
  Set conADO = Nothing
  Exit Sub
 
ErrorRoutine:
  conADO.RollBack
  GoTo Cleanup

[szlamany]

Actually RESUME would normally be used in that case - so that the ERROR TRAP is "turned back on"...

We have a RTN_CALLER: label in all our SUBS and FUNCS and will RESUME that from an ERROR TRAP so that cleanup can occur.

Since it's a standard it's not a spaghetti-code problem for us.

[techgnome]

exactly... that's what the resume is for....Resume, Resume Next, Resume {somelabel}....

-tg

[Bill-E-BoB]

Wow, this thread got big fast. I'm pretty new to the forum & every time I come here I find myself getting more & more impressed with you guys & the info you share. So to answer the questions you've asked me...theres no 'requirement' for me to document everything the code is doing...but our last code-writer quit and I got the dubious honor of replacing him...and his codes are impossible to read, so I had to throw most of 'em away. I label to avoid having that happen. When our mainframe systems are updated I have to update all the codes for abour 40 different macros, so I always make sure I've labeled it in such a way that I can open the code, make the change, and move on...otherwise I'd be stuck there for hours trying to find what I'm modifying.

Why do I use goto statements? Nobody ever told me not to. I've never had a vb training, never read a book, never had someone sit down & say "okay dim means blah, a sub routine does this, and don't use goto statements" (in fact, even now I honestly dont' know what dim means...just what it does). Now that I'm aware of them, I can use private subroutines for each step & keep the code nice & clean in the future...which should definately make life easier for my successors.

As far as the job security thing goes...thats why I came lookin' for you guys. Everything I knew about coding was whatever I could figure out on my own...then I got dumped into a programming position that supports over 50 reps who need my codes...so now I've gotta go from shadetree duct-taped together coding to the real deal. I found a bunch of resources online, bought some books, and found you guys. Already, I've made HUGE improvements in both what I can get done, and how the code looks(I think)...so for that I want to say a big thanks!

[Comintern]

I personally try to avoid having Exit statements inside everything but the simplest functions. I put a label in for the error handler, but don't terminate execution above it. Instead I do an inline error test and let the main code fall through the handler to the common cleanup code:

Private Sub IE()
    
    On Error GoTo ErrorRoutine
    '...
 
ErrorRoutine:
    If Err.Number <> 0 Then conADO.RollBack
    
    If rsADO.State = adStateOpen Then rsADO.Close
    Set rsADO = Nothing
    If conADO.State = adStateOpen Then conADO.Close
    Set conADO = Nothing
  
End Sub

That's just a personal stylistic thing I've got in the habit of doing so that I can ensure that both errored code and normal code get the same cleanup.

[techgnome]

Unfortunately that wouldn't work in our environment as we can and do have multiple error handlers in sections to handle different cases (not all of which result in sub termination).

I think ultimately it comes down to standards.... and consistency. In a one or two person shop it's not as critical as it is in a 20+person shop. We have some very strict standards here to make it easier for new members to pick up on how things are done, and if some one leaves, any one can read their code because we all do things the same way.

In those cases where we need cleanup after handling errors, we Resume to a cleanExit: label.

-tg

[szlamany]

For what it's worth - this example shows our boilerplate convention for every SUB and FUNCTION in our apps. Standard DEBUG.PRINT statements - lead to a meaningful IMMEDIATE WINDOW. We declare 6 longs and 5 strings in every function - even if they aren't going to get used. Default ERROR trapping - it gets enhanced if required, otherwise the standard is left in place...

Works good for us.

Public Function PrintManager(f As Form) As Long
Dim i As Long, j As Long, k As Long, x As Long, y As Long, z As Long
Dim s1 As String, s2 As String, s3 As String, s4 As String, s5 As String
 
Dim booDoReport As Boolean
 
Debug.Print "   PrintManager"
On Error GoTo Error_Handler
 
Begin:
    
    f.MousePointer = vbHourglass
 
    x = f.mintTabNo
    
    booDoReport = False
    
    For i = 1 To gfrmFieldInfo(x, 0, 1)
        If gfrmFieldInfo(x, i, 4) And 16 Then
            booDoReport = True
            Debug.Print "   PrintManager>DoReport=True w/"; gfrmFieldName(x, i)
            Exit For
        End If
    Next i
    
    If booDoReport Then
        PrintManager = PrintReport(f)
    Else
        Call PrintForm(f)
        PrintManager = -1
    End If
    
Rtn_Caller:
 
    f.MousePointer = vbDefault
    
    Exit Function
 
Error_Handler:
    Call Fatal_Error(Err.Number, Err.Source, Err.Description, "PrintManager")
    Resume Rtn_Caller
 
End Function

btw - DIM means DIMENSION - back in the old days you DIM'd an array - other variables were already declared by use.

[Shaggy Hiker]

You can still Dim an array, even in .NET. Redim and Redim Preserve are still with us.

Has little obvious utility for anything other than an array, though. To dimension an array you are stating how many elements it will hold. When you Dim a variable, you state something like the size of the memory needed for that variable. Frankly, I suspect that all variables in .NET are 32 bits long, regardless of what MS says. I think there is nothing but a pointer regardless of whether it is a byte or an integer or a string, but that's a different subject....and one I've never bothered to confirm or refute.

[szlamany]

I meant the really, really old days

Before MS...

[dee-u]

I'd rather let you go and have someone re-write that darn thing... Sorry.

I've hoped you wouldn't have taken that seriously, it's just a funny idea of mine when I remembered something quoted in a member's signature about cluttering your code for job security. I don't use GOTO anymore though I have used it when I was just a noob in Turbo Basic era...

[RhinoBull]

I've hoped you wouldn't have taken that seriously, ...

You have noticed the smilie, didn't you? Was just kidding anyway...