[AdamSpeight2008]

A lot of fellow coders are of the opinion that GOTOs are bad.

• Are there any exceptions though?
  
• Where the usage is permitted?
  
  
• If not show how that code should rewritten as.

---

I give the first suggestion.

How to do a retry?, when the exception is catched.

      If openFileDialog1.ShowDialog() = System.Windows.Forms.DialogResult.OK Then
      Dim myFile = openFileDialog1.FileName
      Dim retrys = 0
      Try
        Using objReader = New StreamReader(myFile)
          Try
            While Not objReader.EndOfStream
JumpBackIn:
              Dim str = objReader.ReadLine.Split("/"c)
              ' output to rich text box
              RTBUnformatedData.Text &= String.Format("{0} {1} {2}", str(1), str(4), str(5)) & vbNewLine
            End While
          Catch ex_OutOfMem As OutOfMemoryException When retrys > 0
            ' Try again upto 3 times.
            If retrys < 3 Then
              objReader.DiscardBufferedData()
              retrys += 1
              ' Jump back to before read.
              GoTo JumpBackIn
            Else
              ' otherwise throw the exception
              Throw ex_OutOfMem
            End If
          Catch ex_OutOfMem As OutOfMemoryException
            ' Discard data and try again
            objReader.DiscardBufferedData()
            retrys = 1
            GoTo JumpBackIn
          Catch ex_IO As IOException
            Throw ex_IO
          Catch ex As Exception
            Throw ex
          Finally
            objReader.Close()
          End Try
        End Using
      Catch Ex As Exception
        MessageBox.Show("Cannot read file " & Ex.Message.ToString)
      End Try
    End If

Do you have a piece of code that demonstrates acceptable usage of GOTO? If so post it.

This post has been edited by AdamSpeight2008: 06 September 2011 - 09:56 AM

[raziel_]

why not just use while? however in VB6 goto ware used to catch exceptions:

On Error GoTo ErrHndl
'Some code here that may create errors'

Exit Function
ErrHndl:
'Catching the error'
End Function

For example function that test if file is opened by other program in VB.NET using GoTo:

    Private Function FileCheck() As Boolean
        On Error GoTo ErrHndl
        Dim SR As New System.IO.StreamReader("D:\test.txt")
        Dim strLine As String = SR.ReadLine()
        Return True
        Exit Function
ErrHndl:
        Return False

    End Function

This post has been edited by NoBrain: 06 September 2011 - 09:05 AM

[Curtis Rutland]

Adam, in your case, I'd suggest extracting a method from that code, and returning a success/fail indicator along with the results of the code execution. This may be by returning a bool and using ref parameters, or it could be done by returning null on failure.

Either way, you could call the method in a loop, and break from the loop if it worked, or if it exceeds the number of retries.

I seriously do not believe there are very many conditions you can come up with where GOTO is going to be the only way to proceed.

[baavgai]

First, we use some functions, to clean up the anonymous code block tied to some form fields.

Function WithGoto(ByVal myFile As String) As String
	Dim retrys = 0
	Dim RTBUnformatedData As String = ""
	Using objReader = New StreamReader(myFile)
		Try
			While Not objReader.EndOfStream
JumpBackIn:
				Dim str = objReader.ReadLine.Split("/"c)
				RTBUnformatedData &= String.Format("{0} {1} {2}", str(1), str(4), str(5)) & vbNewLine
			End While
		Catch ex_OutOfMem As OutOfMemoryException When retrys > 0
			If retrys < 3 Then
				objReader.DiscardBufferedData()
				retrys += 1
				GoTo JumpBackIn
			Else
				Throw ex_OutOfMem
			End If
		Catch ex_OutOfMem As OutOfMemoryException
			objReader.DiscardBufferedData()
			retrys = 1
			GoTo JumpBackIn
		Catch ex_IO As IOException
			Throw ex_IO
		Catch ex As Exception
			Throw ex
		Finally
			objReader.Close()
		End Try
	End Using
	WithGoto = RTBUnformatedData
End Function

' ...
If openFileDialog1.ShowDialog() = System.Windows.Forms.DialogResult.OK Then
	Try
		RTBUnformatedData.Text = WithGoto(openFileDialog1.FileName)
	Catch Ex As Exception
		MessageBox.Show("Cannot read file " & Ex.Message.ToString)
	End Try
End If

I believe that's the spirit of what I'm looking at.

Now, to look again at the function:

Function WithGoto2(ByVal myFile As String) As String
	Dim retrys = 0
	Dim RTBUnformatedData As String = ""
	Using objReader = New StreamReader(myFile)
		Try
			While Not objReader.EndOfStream
JumpBackIn:
				Dim str = objReader.ReadLine.Split("/"c)
				RTBUnformatedData &= String.Format("{0} {1} {2}", str(1), str(4), str(5)) & vbNewLine
			End While
		Catch ex_OutOfMem As OutOfMemoryException ' When retrys > 0
			If retrys < 3 Then
				objReader.DiscardBufferedData()
				retrys += 1
				GoTo JumpBackIn
			Else
				Throw ex_OutOfMem
			End If
		' this is redundent, we just kill the When
		' Catch ex_OutOfMem As OutOfMemoryException
		'	objReader.DiscardBufferedData()
		'	retrys = 1
		'	GoTo JumpBackIn
		
		' also unneeded, we throw a full exception below
		'Catch ex_IO As IOException
		'	Throw ex_IO
		
		' Don't really need this, either, we're catching outside
		'Catch ex As Exception
		'	Throw ex
		
		' We've got a Using, the Close is handled
		'Finally
		'	objReader.Close()
		End Try
	End Using
	WithGoto2 = RTBUnformatedData
End Function

With all the unnessary code removed, we can actually see we don't need the goto at all:

Function WithoutGoto(ByVal myFile As String) As String
	Dim retrys = 0
	Dim RTBUnformatedData As String = ""
	Using objReader = New StreamReader(myFile)
		While Not objReader.EndOfStream
			Try
				Dim str = objReader.ReadLine.Split("/"c)
				RTBUnformatedData &= String.Format("{0} {1} {2}", str(1), str(4), str(5)) & vbNewLine
			Catch ex_OutOfMem As OutOfMemoryException
				If retrys < 3 Then
					objReader.DiscardBufferedData()
					retrys += 1
				Else
					Throw ex_OutOfMem
				End If
			End Try
		End While
	End Using
	WithoutGoto = RTBUnformatedData
End Function

Really, a goto makes sense in the absence of structure programming. It became far less relevant with the advent of procedural programming and there's rarely a case where it improves readability. If anything, it expertly hides key elements that can be refactored.

[Martyr2]

All I have to say about this topic is.... really? I mean really? After all the endless threads about this topic?

[AdamSpeight2008]

baavgai: I think you've slightly misread the function of code in the original post. It should be doing up to 3 retries per a line read, not per a file. (But I've forgotten to reset the count on success).

This one catches the initial exception and set the try count to one

Catch ex_OutOfMem As OutOfMemoryException

The second one with the conditional, catches to others time it is raised.

 Catch ex_OutOfMem As OutOfMemoryException When retrys > 0

The point of the question is how to get back into the while loop? as it no longer in scope when the exception is raised.

[raziel_]

using while?

  Try
                While (retrys < 3)
                    Using objReader = New StreamReader(myFile)
                        Try
                            While Not objReader.EndOfStream
                                Dim str = objReader.ReadLine.Split("/"c)
                                ' output to rich text box
                                RTBUnformatedData.Text &= String.Format("{0} {1} {2}", str(1), str(4), str(5)) & vbNewLine
                            End While
                        Catch ex_OutOfMem As OutOfMemoryException When retrys > 0
                            ' Try again upto 3 times.
                            If retrys < 3 Then
                                objReader.DiscardBufferedData()
                                retrys += 1
                                ' Jump back to before read.
                            Else
                                ' otherwise throw the exception
                                Throw ex_OutOfMem
                            End If
...

[AdamSpeight2008]

Additional
The outer Try...Catch handles only the Exceptions thrown when the Stream is opened.
The inner Try...Catch handles only the Exceptions thrown when the Stream is read.

The Using .. is disposing of the stream object on both success and failure.

You've just created a loop the does not exit on success. (a retry count of 0).

Its 3 retries per a line read, not 3 per a stream.

This post has been edited by AdamSpeight2008: 06 September 2011 - 11:15 AM

[raziel_]

then put the try catch inside the while something like this:

      If openFileDialog1.ShowDialog() = System.Windows.Forms.DialogResult.OK Then
      Dim myFile = openFileDialog1.FileName
      Dim retrys = 0
      Try
        Using objReader = New StreamReader(myFile)

            While Not (objReader.EndOfStream)  And (retrys <3 Or retrys>0)
          Try
              Dim str = objReader.ReadLine.Split("/"c)
              ' output to rich text box
              RTBUnformatedData.Text &= String.Format("{0} {1} {2}", str(1), str(4), str(5)) & vbNewLine
          Catch ex_OutOfMem As OutOfMemoryException When retrys > 0
            ' Try again upto 3 times.
            If retrys < 3 Then
              objReader.DiscardBufferedData()
              retrys += 1
              ' Jump back to before read.
            Else
              ' otherwise throw the exception
              Throw ex_OutOfMem
            End If
          Catch ex_OutOfMem As OutOfMemoryException
            ' Discard data and try again
            objReader.DiscardBufferedData()
            retrys = 1
          Catch ex_IO As IOException
            Throw ex_IO
          Catch ex As Exception
            Throw ex
          Finally
            objReader.Close()
          End Try
            End While
        End Using
      Catch Ex As Exception
        MessageBox.Show("Cannot read file " & Ex.Message.ToString)
      End Try
    End If

i mean is there any reason why the try-catch blog is outside the while?

This post has been edited by NoBrain: 06 September 2011 - 10:46 AM

[AdamSpeight2008]

Reread my previous post.

[baavgai]

Indeed, I must be missing something:

Dim retrys = 0
' ...
' condional catch, wont get hit on the first exception occurance
Catch ex_OutOfMem As OutOfMemoryException When retrys > 0
		If retrys < 3 Then
			objReader.DiscardBufferedData() ' 1
			retrys += 1 ' 2
			GoTo JumpBackIn ' 3
		Else
			Throw ex_OutOfMem
		End If
Catch ex_OutOfMem As OutOfMemoryException
	' this code is functionall identical to the when code above
	' it will be hit only if retrys = 0
	objReader.DiscardBufferedData() ' 1
	retrys = 1 ' 2 - since retrys = 0, the += 1 would make it 1
	GoTo JumpBackIn ' 3

Since retrys is never reset, it will fail after three errors. It does not appear to offer 3 retries per line...

If that's the goal, then an inner check makes even more sense.

Function WithoutGoto(ByVal myFile As String) As String
	Dim retrys = 0
	Dim RTBUnformatedData As String = ""
	Using objReader = New StreamReader(myFile)
		While Not objReader.EndOfStream
			Try
				Dim str = objReader.ReadLine.Split("/"c)
				RTBUnformatedData &= String.Format("{0} {1} {2}", str(1), str(4), str(5)) & vbNewLine
			Catch ex_OutOfMem As OutOfMemoryException
				If retrys < 3 Then
					objReader.DiscardBufferedData()
					retrys += 1
				Else
					retrys = 0
					' Throw ex_OutOfMem
				End If
			End Try
		End While
	End Using
	WithoutGoto = RTBUnformatedData
End Function

That appears to give you the three retries per line you want.

[AdamSpeight2008]

baavgai:

 Catch ex_OutOfMem As OutOfMemoryException
                If retrys < 3 Then
                    objReader.DiscardBufferedData()
                    retrys += 1
                Else
                    retrys = 0
                   ' Throw ex_OutOfMem
                End If

If the line can not be read more than 3 times, your code will loop forever.
As the line is only accessible when the Exception is raised. (Doh!)

retrys = 0

Sequence should.
Exception Raised -> Try Again
Exception Raised -> Try Again
Exception Raised -> Try Again
Exception Raised -> Throw Exception.
At any success reset retries back to zero.

This post has been edited by AdamSpeight2008: 06 September 2011 - 11:13 AM

[baavgai]

But you didn't have reset retrys in the original code! Yes, it does loop forever; you seemed to want that...

So now, perhaps, you want:

Function WithoutGoto(ByVal myFile As String) As String
	Dim retrys = 0
	Dim RTBUnformatedData As String = ""
	Using objReader = New StreamReader(myFile)
		While Not objReader.EndOfStream
			Try
				Dim str = objReader.ReadLine.Split("/"c)
				RTBUnformatedData &= String.Format("{0} {1} {2}", str(1), str(4), str(5)) & vbNewLine
				retrys = 0
			Catch ex_OutOfMem As OutOfMemoryException
				If retrys < 3 Then
					objReader.DiscardBufferedData()
					retrys += 1
				Else
					Throw ex_OutOfMem
				End If
			End Try
		End While
	End Using
	WithoutGoto = RTBUnformatedData
End Function

Nevermind, maybe GOTO is fine in VB...

[raziel_]

oh not that VB shit again
but then again why dont you use try-catch inside the second while?

[AdamSpeight2008]

I already stated that I forgot to reset them

Quote

But you didn't have reset retrys in the original code! Yes, it does loop forever; you seemed to want that...

It is not doing that, so I'll walk you through it. On the forth raised exception (Ie it as failed 3 time before.) It throws the exception "over the wall" to the next outer most exception. where it is then catched by

Catch ex As Exception
  MessageBox.Show("Cannot read file " & Ex.Message.ToString)

The exception has now been consumed, so the stream is disposed of and the method exits.

Quote

but then again why dont you use try-catch inside the second while?

I always have been in this thread, look again.

This post has been edited by AdamSpeight2008: 06 September 2011 - 11:57 AM