Why is using GoTo considered bad practice?

[KitaYama]

But not test the code by
debug.print GoSub xxx

Not that I'm insisting on my way, but I can type above my main sub;
OrderFromFK=123
Gosub GetEmailHeader
Debug.print msg

 

[The_Doc_Man]

@sonic8 For sure you're much better than me in this. So instead of answering your points, I prefer to ask you a question.
Can you explain the benefits of using individual functions when you don't plan to reuse them? (instead of my jump labels)

Thanks.

There is an old principle in problem analysis that is particularly import for programming in ANY language, ABSOLUTELY not limited to VBA. It is called "divide and conquer." This principle is old enough that Julius Caesar used it and explained it in the Commentarii de Bellum Gallicum (Caesar's "Gallic Wars"). That method allowed him to divide and conquer what was Gaul and is now mostly France. TWO THOUSAND YEARS ago. (It is probably an older principle but after a couple of thousand years it gets harder to find the older references.)

The old method that I was taught is that when you deal with problems, you can try to build a huge, monolithic solution, or you can step back and try to divide the problem into various parts. As you sub-divide the problem into progressively smaller steps, you reach the point where many of the steps become trivially easy to code with no further analysis. At that point, the isolated function becomes easy to read and easy to document in whatever way you need to document it. AND whether you will re-use that code elsewhere or not, you will minimize the trouble it gives you.

The idea of a "pool" of public variables at the top of a given module surely works - but makes debugging FAR more difficult, since you have what we USA types call a "free for all" going in, where some code touches something without regard to whatever other code might touch it. The MOMENT that the touch becomes inappropriate, you have Hell to pay in tracing down which routine improperly touched the variable. Forcing the routines to limit themselves to using only the input parameters makes it easier to assure that you DON'T make the inappropriate touches - because remember you divided the problem up into easily understood and totally isolated parts. This structure leads to all kinds of side-effects because the "PUBLIC pool" is equivalently ALWAYS referenced ByRef, never ByVal, so CANNOT be protected against errors.

The difference between GOSUB and straight-out subroutine calls is that a GOSUB simply CANNOT support parameters, so you have no choice but to use "PUBLIC pool" variables and CANNOT constrain those touches.

Here is where things can "go south" on you in a heartbeat... I cannot find any references that suggest that a GOSUB creates a "frame" context. The GOSUB appears to be in the same stack context as its caller. Which means that either

(a) your error handler for the GOSUB's caller must not only handle its own errors, but must also handle errors for ALL of the GOSUB targeted routines, or

(b) each GOSUB routine that needs error handling must declare its own handler BUT when that routine returns to the caller, that calling code must reassert the proper handler. Otherwise, you would have the handler declared in the GOSUB B call handling errors in the GOSUB A context. Yes, this can be done - but is a nightmare of spaghetti code.

If you formalize those GOSUBs into actual subs, they build a proper call frame. Inside that proper context, you can declare a handler if you wish. But the significant part is cleanup. The return from a formal subroutine simply dissolves the call frame on the stack and in so doing, reverts to the prior (caller's) handler automatically. Dissolving the stack frame ALSO dissolves any local variables you might have declared. This is the only time I know where a declared variable actually ceases to exist, and it does so in the name of program cleanup steps.

 

[KitaYama]

Using the GoSub means that all the recordsets are open and no logic is required to determine whether to open or not and no variables have to be passed around. The IDs of parent records are available as they are added so they can be used as FK's in the child records. The array each record gets loaded into is available for each GoSub to use. Using separate subs or functions adds no value and creates complex coupling requirements to pass all that data around. And since the individual record types are irrelevant out of this context, the code has no reuse options.

At last someone understands what I'm talking about.

 

[tmyers]

This turned into quite the conversation! I never received more emails on further replies so I stopped checking but this was all very informative!

 

[Spectate]

In a flowchart goto's are great
This is the logic for the highlight part of a full word search.
Those little 12050 type numbers are "line numbers" another nono in the code.


It's bad but it works

 

[MajP]

Kind Galina: "many-many years ago ... " - when? - first PC is just from 1983 (if I remember correctly)
You are too young to say so, many happy years to you!
...
Have you ever calculated the shortest distance (more then 300 miles on the surface of the Geoid) on the Great Circle Arc by a logarithmic ruler?

Post in thread 'Income Tax Visual Basic' https://www.access-programmers.co.uk/forums/threads/income-tax-visual-basic.315921/post-1744691

 

[gemma-the-husky]

I simplify structures with goto's judiciously. It's a tool in the toolbox.

Sometimes it seems simpler than having to design a loop in a way you can terminate it by testing the loop condition.
It's no different to using an unconditional jump in assembler is it? not that I ever wrote any assembler.

 

[gemma-the-husky]

Unlike most of you, I don't care jumping up and down. I like to create an image of what the whole function is supposed to do. So my code mostly seems like a map. If anyone sees the code, he definitely will understand what I'm trying to do and how.
I like to keep my code clean and tidy. So a train of code from top to down doesn't work for me. I'm not afraid of jumping around like a jack-in-a-box because the editor can be split. The top half of the editor is set to show the map, and the bottom half I can move through different sections of the cod.

Every morning I have to send a mail with an embedded table about previous manufacturing result. The following is a part of the code used for this purpose. I know you pros like you don't like this method, but I absolutely like it.

    Dim Msg As String
    Dim Receiver As String
    Dim Attachment As String
    Dim Subject As String
    Dim CC As String
    Dim AddAttachment As Boolean
    Dim Atta As String
    Dim fltr As String
    Dim rs As DAO.Recordset
 
    If TestMode Then
        Stop
    Else
        On Error GoTo ErrorTrap
    End If
     
    Msg = ""
    GoSub SetFilter
    GoSub AddHtmlHeader
    GoSub AddMailHeader
    GoSub AddTableHeader
    GoSub AddTableRows
    GoSub AddTableEnd
    GoSub GetSignature
    GoSub AddAttachment
    SendMail_Attach True, Receiver, Attachment, Subject, Msg, CC, True, False
   
    Exit sub

Set Filter:
    ......
    ......
    ......
    Return
 
AddMailHeader:
    Msg = Msg & "<p>暲栘恀旤 條<br/>"
    Msg = Msg & "<p>"
    Msg = Msg & DLookup("CompanyName", "tblOrderedFrom", "OrderedFrom_PK=" & OrderedFrom_FK)
    Msg = Msg & "  偐傜怴偟偄拲暥傪捀偒傑偟偨偺偱拲暥彂傪揧晅偟偰傑偡丅<br/>"
    Msg = Msg & "偍朲偟偄強怽偟栿偁傝傑偣傫偑丄彜嵃偺搊榐傪媂偟偔偍婅偄抳偟傑偡丅<br/><br/></p>"
    Return
 

AddTableHeader:
    Msg = Msg & "<table>"
    Msg = Msg & "<tr>"
    Msg = Msg & "<th bgcolor = #E0F8F1 WIDTH='130'>庴拲斣崋</th>"
    Msg = Msg & "<th bgcolor = #E0F8F1 WIDTH='150'>拲斣</th>"
    Msg = Msg & "<th bgcolor = #E0F8F1 WIDTH='150'>惢斣</th>"
    Msg = Msg & "<th bgcolor = #E0F8F1 WIDTH='150'>恾斣</th>"
    Msg = Msg & "<th bgcolor = #E0F8F1 WIDTH='60'>悢検</th>"
    Msg = Msg & "<th bgcolor = #E0F8F1 WIDTH='80' style='text-align:center'>扨壙</th>"
    Msg = Msg & "<th bgcolor = #E0F8F1 WIDTH='120'>婓朷擺婜</th>"
    Msg = Msg & "</tr>"
    Return


AddTableRows:
    ' Create the table rows
    Set rs = CreateRs_DAO("qryOrders_Products", fltr)
    With rs
        If .EOF Then Return
        Do
            Msg = Msg & "<tr>"
            Msg = Msg & "<td bgcolor = #F5F6CE WIDTH='130'>" & !ID & "</td>"
            Msg = Msg & "<td bgcolor = #F5F6CE WIDTH='150'>" & !OrderNo & "</td>"
            Msg = Msg & "<td bgcolor = #F5F6CE WIDTH='150'>" & !ManufacturingNumber & "</td>"
            Msg = Msg & "<td bgcolor = #F5F6CE WIDTH='150'>" & !DrNo & !OrderDrNoChanges & "</td>"
            Msg = Msg & "<td bgcolor = #F5F6CE WIDTH='60'>" & !Quantity & "屄" & "</td>"
            Msg = Msg & "<td bgcolor = #F5F6CE WIDTH='80' style='text-align:right;padding-right:15px'>\" & Format(!UnitPrice, "#,###") &"</td>"
            Msg = Msg & "<td bgcolor = #F5F6CE WIDTH='120'>" & !Delivery & "</td>"
            Msg = Msg & "</tr>"
            .MoveNext
        Loop Until .EOF
    End With
    Return

Anyone who sees the code, can imagine how it works. If I need to change some section of it, I can jump right to the appropriate code.
As I said, the first GoSub Section works like a map for the code. It tells me what I'm trying to do in each step. If I need to change the sequence, I simply can drag a GoSub to a higher or lower place.

I do things like that. It's a convenient resource, and easy to read. A problem with VB is that all the subs share the variables in the procedure, so you can't (officially) make it even more bulletproof by passing arguments to the sub - although you can sort of do that, by having designated variables for each sub.

 

[gemma-the-husky]

Honestly (I'm being sincere, not sarcastic) the easiest way to become convinced to agree with avoiding using it is probably to do this:

Begin using it liberally in code that you're still working on developing. Something that won't be super simple but will evolve a bit and become at least moderately complex. After a while you'll become a believer.

It's really not that there is anything "bad" about a single GoTo. The problem is that GoTo's as a logical construct tend to immediately begin spawning. 1 will turn into 3, and all of a sudden you'll realize that the 3 must become 15 in order for everything to work. Trust me, it becomes a nightmare slippery slope.

I use them once in a great while, but normally there are better ways to design your control-of-flow.

Then again, improving the quality of your control-of-flow is an infinite life's work, as we are always getting better and improving.

For example I personally DO use Exit Function pretty liberally in my functions, while that is recommended to avoid (in favor of taking more time to think it through but I get lazy sometimes).

I wouldn't use goto's to provide a process framework.
I would set up a module with gosubs in the way that @KitaYama demonstrated, so you build your flow with placeholders for the subs you need. (which you can then replace with subs/functions if necessary as you expand on the subs)

 

[gemma-the-husky]

Well let's say you are iterating a recordset of some sort to produce an output file. (eg EDI structured file, XML file, JSON file)

If you use procedures for each type of output line, you need to pass in to the function/procedure a recordset variable, and return a string to add to the file., or pass in the file index and write the output string to the file in each procedure you call.

If you use gosub's it's all in the same single procedure, and rather easier to manage in many ways.

Either way, the real problem is iterating the data in an order that matches the structure of the file you want to produce.

eg this sort of thing

open resources
batch header
     invoice header(s)
         invoice line(s)
     invoice footer
batch footer
close resources

 

[The_Doc_Man]

With regard to "many moons ago" ... The PC might not be that old, but my first computer was an IBM 1620 Mk II, which I first touched in 1966. Since it used a flavor of FORTRAN (II-D), it certainly had GOTO statements. It was an early procedural language but that flavor didn't have loop-exiting methods equivalent to EXIT DO or such. That didn't come until later versions like FORTRAN-IV or other contemporary languages.

As Dave suggested earlier, "GOTO" is a tool in the toolbox, perhaps to be used rarely, but to be used. I think I have a long-enough perspective to say that the advent of more advanced procedural languages with other ways to break out of loops is what led to the denouncement of GOTO statements. However, there is another viewpoint about why Nicklaus Wirth (father of PASCAL) was so against them: determination of program computability.

If we recall the work by Alan Turing, he got involved with automata theory and was at one point delving into finite vs. non-finite automata. Wirth did some work as well in that topic and found that addition of GOTO statements could lead to situations where the finite or non-finite nature of the overall algorithm (i.e. the "automaton") could not be determined. (The actual "no-no" was a non-consuming state transition, a change of a program from one state to another, without consuming some form of input. It's an abstract form of a GOTO.) This is because a GOTO can jump ANYWHERE and can disrupt flow issues. From where do you think language authors determined that you cannot jump into the middle of a loop or the middle of an IF block or into the middle of a GOSUB routine? Theory has shown that those (among other operations) completely destroy the ability of the computer to perform a predictable operation. If you have an advanced flow-checker program, GOTO makes life really icky-tricky.

We are used to assertoric logic (A implies B, B implies C, therefore A implies C... that kind of logic) but there is something else called interrogative (or sometimes interrogatory) logic. Interrogative logic asks whether a question is answerable and if the answer can possibly be right. (Does the question "condone" a given answer?) It was a VERY big contributor in analysis of set theory as it applies to queries.

The presence of a GOTO in code is one of those factors that throws the "monkey wrench" into determining answerability. Note that "standard" SQL only has IMPLIED loops, no explicit ones, and no GOTO operations. You can have a GOSUB of sorts by having a sub-query or by using a query as the data source of a FROM statement (i.e. nested queries). But SQL is in a way the ultimate result of the computability concept.

Based on the Goedel Completeness Theorem, a language can be complete or correct but cannot be both. SQL is "correct" whereas languages that allow GOTO are "complete" (more or less). What Goedel was saying is that complete languages can create self-contradictory statements or conditions. Correct ones cannot. SQL only does assignment statements via the ALIAS concept, so you can't define something in an extraordinary way, unlike languages that can do odd things to their variables. Standard SQL is "define it ahead of time or don't use it at all." Those variants of SQL that include some sort of procedural language in-line with SQL may "break" the computability.

Access VBA doesn't break SQL's computability because it does not and cannot co-exist directly with SQL. If you want to put something in an SQL statement you have to build it as a string first and then pass it to your SQL processor (ACE, SQL Server, etc.) We often moan and groan about VBA and SQL and how they don't always work so closely, but it turns out that there is a good reason for it. I'll be kind and presume that it was a matter of intent that the VBA and SQL environments DON'T overlap so that it becomes possible to validate computability.

 

[gemma-the-husky]

@Pat Hartman

Sorry Pat, I didn't realise your earlier example was exactly the same thing - I just thought it was analogous.

All of these constructs are provided to assist, aren't they? instr() was good enough to find the last instance of something, but a bit of a pain to write your own loop of instr() calls, then instrrev() came along.

One construct I do find awkward is returning multiple values from a function. I think this is mainly from being conditioned not to change passed in arguments and to just use the function return value - but if you want to return multiple values from a function you have to do so if you want the function to be self-contained.

So there's a lot of alternatives to manage a process - a goto, a gosub, functions with no return value, functions with a return value, procedures, using public variables, and as long as it works, and it's understandable, I think it's all fair game.

 

[KitaYama]

One construct I do find awkward is returning multiple values from a function.

@gemma-the-husky
How about this : You have 3 returning values from one function.

Sub test()
    Dim Param1 As Date
    Dim Param2 As String
    Dim Param3 As Integer
   
    Param1 = Date
    Param1 = MyFunction(Param1, Param2, Param3)
    Debug.Print Param1
    Debug.Print Param2
    Debug.Print Param3
End Sub

Public Function MyFunction(Param1, Optional ByRef Param2, Optional ByRef Param3) As Date  
        Param2 = "Kita Yama"
        Param3 = 100
        MyFunction = DateAdd("d", 1, Param1)       
End Function

 

[jdraw]

Regarding returning multiple values from one function,
see this post/example by MajP

 

[KitaYama]

Regarding returning multiple values from one function,
see this post/example by MajP

I personally prefer not to work with public variables.

 

[cheekybuddha]

I personally prefer not to work with public variables.

It's not a public variable. It's a user-defined type.

 

[KitaYama]

I know but doesn't user-defined type reset because of an un-handled error?

 

[cheekybuddha]

No, it doesn't hold any value.

In the example, the variables of the UDT are defined locally (ie the return value of function EmpWNIS() and Dim TN As TableNIS in the Test() sub)

 

[cheekybuddha]

Some more classic examples by ChrisO here

 

[gemma-the-husky]

@gemma-the-husky
How about this : You have 3 returning values from one function.

Sub test()
    Dim Param1 As Date
    Dim Param2 As String
    Dim Param3 As Integer
 
    Param1 = Date
    Param1 = MyFunction(Param1, Param2, Param3)
    Debug.Print Param1
    Debug.Print Param2
    Debug.Print Param3
End Sub

Public Function MyFunction(Param1, Optional ByRef Param2, Optional ByRef Param3) As Date
        Param2 = "Kita Yama"
        Param3 = 100
        MyFunction = DateAdd("d", 1, Param1)     
End Function

Yes - I know how to do it, it just feels slightly wrong when I use arguments as byRef