views:

412

answers:

11

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(using if statements checking the integer inputs size, if there's a better way to do this, I'd appreciate if you could tell me that also :D) How would I be able to loop back to ask the user again?

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
A: 
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
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
`break` and `continue` are just more specific versions of GOTO :)
Earlz
+2  A: 

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

Procedural / Functional programming is a much better approach.

Raj More
You should phrase that "The GOTO construct CAN produce spaghetti code".
erikkallen
+1  A: 

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
A: 

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
+4  A: 

http://xkcd.com/292/ i think this is the standard opinion of Goto.

instead try and use a do while loop. do while loops will always execute once and are great when you need to propmt the user, but make sure they enter the correct information.

Sub Main()
    Dim valid as Integer
    valid=0
    Do Until valid = 1
    System.Console.WriteLine("enter the day ")
    day = System.Console.ReadLine()
    If day > 31 Or day < 1 Then
       System.Console.WriteLine("Invalid day\n")
       valid = 0;
    Else
       valid = 1

    Loop

End Sub
Andy
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
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
+2  A: 

Questions about the merits of the GoTO statement (or rather the lack thereof) are perennial on this site. Look for 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, mabye 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
mjv
+7  A: 

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.. )

Valid=false
While not Valid
    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")
    Else
        Valid=true
    End If
EndWhile

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"

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"

Earlz
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
+1  A: 

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
A: 

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
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
"here" in "the example here" refers to the example in the question.
Joshua
A: 

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
-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
+1  A: 

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
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