10

I've posted a code snippet on another forum asking for help and people pointed out to me that using GoTo statements is very bad programming practice. I'm wondering: why is it bad?

What alternatives to GoTo are there to use in VB.NET that would be considered generally more of a better practice?

Consider this snippet below where the user has to input their date of birth. If the month/date/year are invalid or unrealistic, I'd like to loop back and ask the user again. (I'm using if statements to check the integer's size... if there's a better way to do this, I'd appreciate if you could tell me that also :D)

retryday:
    Console.WriteLine("Please enter the day you were born : ")
    day = Console.ReadLine
    If day > 31 Or day < 1 Then
        Console.WriteLine("Please enter a valid day")
        GoTo retryday
    End If
Harry Johnston
  • 35,639
  • 6
  • 68
  • 158
qais
  • 113
  • 2
  • 2
  • 6

14 Answers14

21

I'm going to differ from everyone else and say that GOTOs themselves are not all the evil. The evil comes from the misuse of GOTO.

In general, there is almost always better solutions than using a GOTO, but there really are times when GOTO is the proper way to do it.

That being said, you are a beginner, so you shouldn't be allowed to judge if GOTO is proper or not (because it hardly ever is) for a few more years.

I would write your code like this (my VB is a bit rusty...):

Dim valid As Boolean = False

While Not valid
    Console.WriteLine("Please enter the day you were born: ")

    Dim day As String

    day = Console.ReadLine

    If day > 31 Or day < 1 Then
        Console.WriteLine("Please enter a valid day.")
    Else
        valid = True
    End If
End While

If you take your GOTO code and look at it, how would someone first approach your code? "Hmm.. retryday? What does this do? When does this happen? Oh, so we goto that label if the day is out of range. Ok, so we want to loop until the date is considered to be valid and in range".

Whereas if you look at mine:

"Oh, we want to keep doing this until it's Valid. It is valid when the date is within range."

djv
  • 15,168
  • 7
  • 48
  • 72
Earlz
  • 62,085
  • 98
  • 303
  • 499
  • 1
    Would like to add, use the goto to avoid convoluted logic constructs. Note that using goto in VB.NET can generally be avoided. In C#, you either use goto or sprinkle return statements throughout the code. – AMissico Apr 24 '10 at 21:28
  • @AMissico: for god's sake, don't use goto to avoid using a return statement. That would qualify as an evil use of goto. The principle here is simple: when a "goto" is encountered, the reader has to follow the goto to know what is going on. When a more explicit construct, such as return is seen, the reader immediately knows exactly what is going on. And knows that the coder did not do anything potentially stupid. In addition, what starts as a trivial use of goto can become less obvious, as a method is added to over time. – ToolmakerSteve Nov 21 '13 at 01:31
  • @AMissico: re C# vs VB.NET: C#, since Day 1, has had an equivalent to every flow-control construct that VB.NET has. Also, goto is NEVER necessary in either language. Factor out a method (even if only called from one place) and use returns. Use While/Until loop with a boolean flag. – ToolmakerSteve Nov 21 '13 at 01:39
  • @ToolmakerSteve. What are you talking about? Are you sure you are responding to my small comment? – AMissico Nov 21 '13 at 08:21
  • @ToolmakerSteve. You seem to be implying that using `return` statements as flow-control statements is okay. Yet, in my opinion, that would qualify as evil use of `return`, makes code difficult to read, and increases method complexity. – AMissico Nov 21 '13 at 08:34
  • @AMissico: Yep, using return statements as flow-control is preferable to the alternatives. Properly used, it DECREASES code complexity, and maintains ease of readability. It becomes a problem when someone nests deeply, so that the return is buried deeply. In that case, the problem is the deep nesting: any other solution will be equally hard to read, unless the deep part is factored out as a separate method. Another problem occurs if the return is stuck at the end of a line after another statement (in languages that allow that). return on its own line - very clear, and easy to read. – ToolmakerSteve Nov 27 '13 at 19:21
  • @AMissico: re C# vs VB.Net, I'm responding to your sentence "Note that using goto in VB.NET can generally be avoided. In C#, you either use goto or sprinkle return statements throughout the code." The implication is that VB.Net can avoid goto in situations that C# cannot. That is not true; C# has equivalents to all of VB's control constructs. – ToolmakerSteve Nov 27 '13 at 19:25
  • @ToolmakerSteve, from experience, using multiple return statements is a mess and should be avoided. Every return statement increases maintenance complexity. That is a fact. – AMissico Nov 27 '13 at 20:30
  • @ToolmakerSteve, Yes C# has the same equivalents and a few more. VB programmers always contructed their logic and procedures to have one return due to the nature of the language. This practice naturally moved to VB.NET. Therefore, it common place to avoid certain control flow statements by keeping procedures small with single exit points. This is not so in the C-like languages. – AMissico Nov 27 '13 at 20:36
  • @AMissico. We can't usefully resolve this here. Would have to show examples and counter examples. The point in each situation, is what you do INSTEAD of the return. Return is, of course, far down on the list of solutions to use. And I never use a return except in a short method, no more than two indentations deep. However there are situations where the alternatives are worse. "goto" is even farther down on the quality list, for the reason I gave in my initial response. Much of my career has been maintaining and extending other people's crappy code; I am quite familiar with abuse of return. – ToolmakerSteve Nov 27 '13 at 20:37
  • @AMissico: here is the point that puzzles me: Are you saying that goto a label at the end of a function is "less maintenance complexity" than a return, which is the equivalent? If so, then why? If not, then what are you saying, about the use of goto vs. return? – ToolmakerSteve Nov 27 '13 at 20:51
  • @ToolmakerSteve; >>We can't usefully resolve this here.<< I think we would agree more than we disagree. It seems we just stumbled on some items due to lack of context through "comments" only. >>I am quite familiar with abuse of return<< I inherited a big ball of crap a few years ago. I know exactly what you are talking about. – AMissico Nov 28 '13 at 01:34
  • @ToolmakerSteve; >>puzzles me<< I am not sure. I wonder what the "Maintainability Index" of three identical methods would be with different control flow statements? One method uses a `switch` and single return variable, another switch-like `goto` and a single return variable , and a third with a `switch` and multiple return variable. I would imagine all three would compile to nearly identical IL. If the methods just called `return` at various points without returning a variable, I suspect the IL would be much different between the three. – AMissico Nov 28 '13 at 02:18
6

http://xkcd.com/292/ I think this is the standard opinion of GoTo.

Instead, try and use a Do Until loop. Do Until loops will always execute once and are great when you need to prompt the user and want to make sure that you do not proceed until they enter the correct information.

Sub Main()
    'Every time the loop runs, this variable will tell whether
    'the user has finally entered a proper value.
    Dim Valid As Boolean = False

    'This is the variable which stores the final number which user enters.
    Dim Day As Integer = 0
    Do Until Valid
        Console.WriteLine("Enter the day:")
        Dim DayStr As String = Console.ReadLine()

        If Not Integer.TryParse(DayStr, Day) Then
            Console.WriteLine("Invalid value! It must be a valid number.")
            Valid = False
        ElseIf (Day < 1) Or (Day > 31) Then
            onsole.WriteLine("Invalid day! It must be from 1 to 31.")
           Valid = False
        Else
           Valid = True
        End If
    Loop

    'blablabla
    'Do whatever you want, with the Day variable
End Sub
Sreenikethan I
  • 319
  • 5
  • 17
Andy
  • 111
  • 7
  • Why use an Integer instead of a Boolean in this case? In my mind it's cleaner to write 'Do Until valid' Also, there is no need to set valid to 0 since it is already 0. – Chris Dunaway Apr 26 '10 at 17:38
  • I used an int because i wasn't sure if VB had a boolean class. I declared valid = 0 because you should always make sure variables are explicitly declared before you reference them. – Andy Apr 27 '10 at 22:28
  • @ChrisDunaway I guess he is not a native VB programmer? I believe some programming languages do not have Booleans, hence the 0 and 1. And also, you can see the semicolon in `valid = 0;` – Sreenikethan I Apr 08 '20 at 16:32
2

The GOTO construct produces sphagetti code. This makes tracing through code almost impossible.

Procedural / Functional programming is a much better approach.

Raj More
  • 47,048
  • 33
  • 131
  • 198
2

Questions about the merits of the GoTo statement (or rather the lack thereof) are perennial on this site. Click here for an example: Is GoTo still considered harmful?

With regards to an alternative to GoTo, in the provided snippet, a while loop would nicely do the trick, maybe something like:

day = -1
While (day < 0)
   Console.WriteLine("Please enter the day you were born : ")
   day = Console.ReadLine
   If day > 31 Or day < 1 Then
     Console.WriteLine("Please enter a valid day")
      day = -1
   End If
End While
Community
  • 1
  • 1
mjv
  • 73,152
  • 14
  • 113
  • 156
1

Functions FTW!

Okay, I'm not sure if your code is really VB.Net here, since you've got some wonky type stuff going on (i.e. Console.Readline returns a String, not a number you can do comparisons on)... so we'll just forget about type for the moment.

Console.Writeline("Please enter the day you were born : ")
day = Console.Readline()

While not ValidDate(day)
   Console.WriteLine("Please enter a valid day")
   day = Console.Readline()
End While

And separately

Function ValidDate(day) As Boolean
  Return day > 31 Or day < 1
End Function

Or you could have fun with recursion and early-return syntax! ;)

Function GetDate() As String
  Console.Writeline("Please enter the day you were born : ")
  day = Console.Readline()

  If ValidDate(day) Then Return day 'Early return

  Console.Writeline("Invalid date... try again")
  GetDate()
End Function
Jeff B
  • 8,572
  • 17
  • 61
  • 140
  • 1
    Don't use recursion, unless there is no simple way to solve with loop. One problem: many methods become more complicated over time, as other conditions are thought of. An edit, years later, might accidentally convert a simple use of recursion, into a potential infinite recursion. Another problem with recursion: call stack gets deeper. Not likely to matter here, but its a bad habit. Oops, I used recursion because I was lazy or clever, not realizing that it was a situation that would become thousands of calls deep... and a later edit made it too complicated for the compiler to tail recurse. – ToolmakerSteve Nov 21 '13 at 01:24
1

GOTOs are a pretty political issue. The 'solution' to GOTOs is to use other built-in navigation constructs like functions, methods, loops, etc. For VB, you could make a sub-procedure that runs that code, or put it in a While loop. You can google both of those subjects fairly easily.

brydgesk
  • 864
  • 1
  • 8
  • 14
1

A little clunky but:

    Dim bContinue As Boolean

    Console.WriteLine("Enter a number between 1 and 31")

    Do
        Dim number As Integer = Console.ReadLine()
        If number >= 1 AndAlso number <= 31 Then
            bContinue = True
        Else
            Console.WriteLine("Please enter a VALID number between 1 and 31")
        End If
    Loop Until bContinue

Also consider some basic loops in "goto land"

        Dim i As Integer
startofloop1:

        Debug.WriteLine(i)
        i += 1
        If i <= 10 Then
            GoTo startofloop1
        End If

        i = 0

startofloop2:

        Debug.WriteLine(i * 2)
        i += 1
        If i <= 10 Then
            GoTo startofloop2
        End If

Here's the nice equivalent:

   For x As Integer = 0 To 10
        Debug.WriteLine(i)
    Next
    For x As Integer = 0 To 10
        Debug.WriteLine(i * 2)
    Next

Which is more readable and less error prone?

Jeremy
  • 4,808
  • 2
  • 21
  • 24
1

Using goto has been considered a bad practice for decades now. Perhaps it was a backlash against the original BASIC (before Visual Basic). In the original BASIC there were no while loops, no local variables (only globals), and (in most BASIC versions) functions could not take parameters or return values. Moreover, functions were not explicitly separated; control can implicitly fell from one function to another if you forgot a RETURN statement. Finally, code indentation was a foreign concept in these early BASICs.

If you used the original BASIC for a some time (like I did), you would come to appreciate how the use of global variables and gotos everywhere makes a large program hard to understand, and without great care, turned it into a tangled mess of "spaghetti". When I learned QBASIC, with its WHILE..WEND loops and SUBs, I never looked back.

I don't think gotos hurt in small quantities, but in the coder culture a strong sense lingers that they are somehow evil. Therefore, I would avoid gotos for no other reason than to avoid offending sensibilities. Occasionally I find that a goto solves a problem cleanly (like breaking out of an outer loop from within an inner loop), but you should consider whether another solution makes the code more readable (e.g. put the outer loop in a separate function and use "exit function", instead of goto, in the inner loop).

I wrote a C++ program with perhaps 100,000 lines of code and I've used goto 30 times. Meanwhile, there are more than 1,000 "normal" loops and around 10,000 "if" statements.

Qwertie
  • 16,354
  • 20
  • 105
  • 148
  • My old computer teacher use to write in Basic "We didn't indent back then of course. We saved all the memory we could get! We even made a program so that it would compress our BASIC programs as much as possible removing all new lines and shortening variable names!" – Earlz Apr 23 '10 at 21:21
0

I'll throw mine even though the 'by the book' wolves outhere will downvote. Have a look at : Is it ever advantageous to use 'goto' in a language that supports loops and functions? If so, why?

Community
  • 1
  • 1
lauCosma
  • 154
  • 10
0
While True
    Console.WriteLine("Please enter the day you were born : ")
    day = Console.ReadLine
    If day > 31 Or day < 1 Then
        Console.WriteLine("Please enter a valid day")
        Continue
    Else
        Break
    End If
End While
Andrey
  • 59,039
  • 12
  • 119
  • 163
  • 1
    Ewwwww, how about making that a do loop instead? More like DO ... Loop while day > 31 Or day < 1. Removes the continue and the else case. – tloach Apr 23 '10 at 17:22
  • 2
    `break` and `continue` are just more specific versions of GOTO :) – Earlz Apr 23 '10 at 17:31
  • 1
    @Earlz. Apologies if that was an attempt at humor; I'm taking your comment literally: The whole point of constructs like "break" and "continue" is that they do well-defined, known-to-be-safe-and-understood jumps. Whereas if a goto is encountered, the person reading has to manually verify that the coder didn't do something stupid. Furthermore, if you see "goto", you don't know the REASON for the branch: what does it accomplish? Whereas if you see "break" or "continue", you know immediately what it does, and why. – ToolmakerSteve Nov 21 '13 at 01:05
0

You can do almost anything that you can do with GOTOs with simple built-in language constructs like decision structures and loops, and GOTO statements often make for messy, impossible-to-understand spaghetti code. Loops and ifs and such have a clear, accepted, understandable usage.

See, as is usually suggested, Dijkstra's Go-To Statement Considered Harmful

froadie
  • 79,995
  • 75
  • 166
  • 235
0

It is often recommended that we follow Dijkstra's advice in Go-To Statement Considered Harmful.

Donald Knuth answered Dijkstra pretty soundly. This example here is a modern version of one of his counterexamples. Personally I write infinite loops with internal breaks when I encounter this one but there's a few other rare cases where I will write GOTO statements.

The most common ones for me are breaking out of deeply nested loops and this pattern:

ContinueTry:
   Try
        'Worker code
   Catch ex as IO.IOException
        If MessageBox.Show(...) = DialogResult.Retry Then Goto ContinueTry
        Throw
   End Try

I also have two cases of large finite state machines with goto statements providing the transitions.

Joshua
  • 40,822
  • 8
  • 72
  • 132
  • 1
    A trivial transform removes the goto from this example. A better example is: how do you break/continue an outer loop from within an inner loop? – Qwertie Apr 23 '10 at 20:51
  • "here" in "the example here" refers to the example in the question. – Joshua Apr 25 '10 at 01:20
0

I've got to agree with everyone else here: GOTO itself is not evil, but misusing it will certainly make your life miserable. There are so many other control structures to choose from, and a well-written program can usually handle most every situation without goto. That being said, I am at the near-completion point of a program that's racking up about 15,000 lines, and I used one, and only one, GOTO statement (which I may be replacing we will see). It's the first time I've used GOTO in the last dozen or so programs I've dealt with. But in this instance it got rid of a compiler error (using Me.Close() twice within the same Sub but within different If structures; I could have suppressed it but I simply threw in a label and replaced one Me.Close() with a GoTo CloseLabel). If I start running into more instances that require Me.Close() within this Sub, I'm likely to put Me.Close() in its own sub and simply call that sub from the If structures or other loops that result in a closing of the program... As I said, there's alternatives, but sometimes, and when used very rarely, sparingly, and strategically, GoTo can still be helpful. Just beware of spaghetti code, that's a blinking mess lol

Michael Tant
  • 25
  • 1
  • 5
  • I don't think the original question is a good fit for Stack Overflow but your answer is suitable given the question. Maybe you might want to break this answer up into separate paragraphs instead of a giant blob of text to make it more readable. – Kmeixner May 09 '16 at 13:28
-2

Your code is fine. It is concise and clear. It is better than inflating the job 50% to 200% with extra variables and different verbs that do the same thing.

If you are just skipping backwards or forwards to the beginning or end of a logical block, then go(to) for it. A "Loop" or an "End while" is still a goto, but the destination is implied. The only advantage is that the compiler will stop you from making two loops cross paths, but it can't with a pair of gotos. When using goto's, don't cross the streams. That would be bad. -- Dr. Spengler

My other pet peeve is the "one entrance, one exit" rule. Of course you can only have one entrance, unless you are writing in assembler. But the "one exit" rule is stupid. It just leads to a bunch of nested bounds checks that walks your code off the right margin. It is much more clear to test all your parameters at the top of the routine and "exit sub" if they are illegal. What makes more sense?

if badparam then
  log error
  exit sub
  endif

if badparam2 then
  log error2
  exit sub
  endif

do stuff

or this?

if goodparam then
  if goodparam2 then
    do stuff
  else
    log error2
    endif
else
  log error 
  endif 

When you have six bounds checks and "stuff" is 60 lines that you can't break up into smaller bits, then the second way turns into a nightmare for anyone who has to maintain it. It's better to finish what you were doing -- checking the exceptions -- than to defer all the exception handling to the end.

My $0.02

Ron
  • 269
  • 1
  • 6
  • -5 for saying "Goto in that context is definitely better than an explicit loop" +1 for the anti 1-exit rule. (I have no idea why 1 exit is considered more "structured" than a couple exits. Of course, it can always be abused too though) – Earlz Apr 23 '10 at 20:20
  • 1
    The rule I follow for exits is is: "Avoid using `return` between the first and last statements in a function that have side-effects". – supercat Oct 01 '12 at 16:57
  • I also minus 1'd the response. This is the first time I've ever felt a response merited the harshness of -1. This is definitely NOT one of the handful of situations where a GOTO is justifiable. Most beginning programmers would be better off if they learned to NEVER use a goto, so that they were forced to understand how to refactor code so that it is easy to understand. Only then, after two or three years of programming, should they consider using a goto. NOTE: On the other hand, I 100% agree that "one exit" is an insane rule, that leads to unnecessary complications in streamlined code. – ToolmakerSteve Nov 21 '13 at 01:11
  • @ToolmakerSteve: Just because you heard of Edsger Dijkstra's GoTo Considered Harmful article does not mean that every use of goto is evil. Take a look at http://stackoverflow.com/questions/24451/is-it-ever-advantageous-to-use-goto-in-a-language-that-supports-loops-and-func/2809622#2809622 and we will talk about it. I've heard enough of 'goto is bad practice'. – lauCosma Oct 16 '14 at 13:55
  • +1 for expressing your opinion and not reading like a 'machine' – lauCosma Oct 16 '14 at 13:56