Good Programming practice- using goto

[HanneSThEGreaT]

I don't agree xtab

Here, a variable was set to either True or False, and that variable gets returned. By using the variable it is the same as saying Return True or Return False. It is even the same as saying TestConnection = True or TestConnection = False

technicalities....

[GremlinSA]

you have missed return of any function.

Granted, I'll give you that.. but what about if it was written like this.. EDIT: I (mis)understood this statement as saying 'the Catch block still has code in it'

        Public Shared Function TestConnectionString(ByVal ConnectionString As String) As Boolean
            Dim Reply As Boolean = False
            Try
                Dim sqlConn As New SqlConnection(ConnectionString)
                sqlConn.Open()
                Reply = True
                sqlConn.Close()
            Catch ex As Exception
            End Try
            Return Reply
        End Function

This is also 'Good' programming, and here there is nothing in the catch block.. However the result is the same..

But now going back to the OP one of the better methods to use is something like this.

    Public Sub SelectAfile()
        Dim FileOK As Boolean = False
        Dim OpenCancel As Boolean = False
        Dim Result As DialogResult
        Do
            'Show Open File Dialog
            Result = OpenFileDialog1.ShowDialog()
            If Result = Windows.Forms.DialogResult.OK Or Result = Windows.Forms.DialogResult.Yes Then
                'Check if file satisfactory
                If OpenFileDialog1.FileName = "HappyFile.ext" Then
                    FileOK = True
                End If
            Else
                'User Cancled out
                OpenCancel = True
            End If
        Loop Until FileOK Or OpenCancel


        If FileOK Then
            'We can now process the file ...

        End If

    End Sub

there are no Nasty GOTO's and an easy to follow sub..

[GremlinSA]

you have missed return of any function.

public Shared Function TestConnectionString(ByVal ConnectionString As String) As Boolean
            Dim Reply As Boolean
            Try
                Dim sqlConn As New SqlConnection(ConnectionString)
                sqlConn.Open()
                return True
                sqlConn.Close()
            Catch ex As Exception
               Return false
            End Try
            Return Reply
        End Function

Let me also just add that writing it like this is 'VERY BAD' Practice...

'RETURN X' is the same as using 'EXIT FUNCTION' but sends the variable 'X' as the return value..
In this case if the connection did not fail, the connection would stay open as 'sqlConn.Close()' will never be called...

And like TechGnome put it.. I subscribe to the Single-Entry, Single-Exit design when it comes to Subs/Methods/Functions... This is solid advice...

[TT(n)]

one of the better methods to use is something like this

       Do
            'Show Open File Dialog
            Result = OpenFileDialog1.ShowDialog()
            If Result = Windows.Forms.DialogResult.OK Or Result = Windows.Forms.DialogResult.Yes Then
                'Check if file satisfactory
                If OpenFileDialog1.FileName = "HappyFile.ext" Then
                    FileOK = True
                End If
            Else
                'User Cancled out
                OpenCancel = True
            End If
        Loop Until FileOK Or OpenCancel

Ooo, well one of but not really in the particular case of dialogs.
I must reiterate that the thread is already resolved with the most proper solution on post #6. It's seems that this is the second time the resolution was overlooked. No?

Actually dialogs don't like to be called in a loop, and can in fact wonk out on you with spurious results. I've had it happen in several projects, that is until I discovered the proper way to cancel the ok(open) button before it's processed.

Thus there is no need to repeat, Do over, or Go Back To, since you cancelled it from ever happening(but leaves the dialog still open!). If there were no ability to cancel, then a loop might be an easy route, but trust not it's stability.

[TT(n)]

public Shared Function TestConnectionString(ByVal ConnectionString As String) As Boolean
            Dim Reply As Boolean
            Try
                Dim sqlConn As New SqlConnection(ConnectionString)
                sqlConn.Open()
                return True
                sqlConn.Close()
            Catch ex As Exception
               Return false
            End Try
            Return Reply
End Function

I certainly agree that is not even practice, nevermind good practice, however I think you can use Finally to close it?

I guess practice depends alot on the habit level of the individual, and if one trusts oneself to be careful enough. Forever changing as one grows.
I myself am very mindful about the paths of execution, and so I may tend to exit whenever I feel it's really ok, or shorter code etc. I can see the point of view of a "Single exit" strategist though. I do get it.

[JohnnyD]

you have missed return of any function.

public Shared Function TestConnectionString(ByVal ConnectionString As String) As Boolean
            Dim Reply As Boolean
            Try
                Dim sqlConn As New SqlConnection(ConnectionString)
                sqlConn.Open()
                return True
                sqlConn.Close()
            Catch ex As Exception
               Return false
            End Try
            Return Reply

        End Function

Also agree this is incredibly bad as the sqlConn.Close() line would never get executed.

As a side-note to this, I have read that you should never close a database connection in a Finally block because if an exception is thrown in the Catch block, the Finally block will never execute.. ie:

        Dim cn As New SqlConnection(connectionString)
        Dim cmd As New SqlCommand
        Dim trans As SqlTransaction

            Try

                'start the transaction and save the batch details first

                cn.Open()
                trans = cn.BeginTransaction

                With cmd
                    .Connection = cn
                    .Transaction = trans
                    .CommandType = CommandType.StoredProcedure
                    .CommandText = "usp_Batches_Save"

                    With .Parameters
                        ' Add some parameters
                    End With

                    .ExecuteNonQuery()

                End With

                ' if there are orders associated with the batch then update the batch number of
                ' each order.

                If Not sBatch.Orders Is Nothing Then
                    For i = 0 To sBatch.Orders.Length - 1

                        cmd = New SqlCommand
                        With cmd
                            .Connection = cn
                            .Transaction = trans
                            .CommandType = CommandType.StoredProcedure
                            .CommandText = "usp_Batch_Order_Save"
                            .Parameters.Add("@link_id", sBatch.Orders(i))
                            .Parameters.Add("@batch_no", batchNo)
                            .ExecuteNonQuery()
                        End With
                        cmd = Nothing
                    Next
                End If

                trans.Commit()
                trans = Nothing
                cn.Close()

            Catch ex As Exception

                trans.Rollback()
                trans = Nothing

                cn.Close()

                Throw ex

            End Try

In the above code, the connection will always close if there is an error but if I had done this:

                trans.Commit()
                trans = Nothing

            Catch ex As Exception

                trans.Rollback()
                trans = Nothing

                Throw ex

            Finally
                cn.Close()

            End Try

and an error occured when the transaction was rolled back (for example) then the Finally block would never execute and the connection would not get closed.

Just something I read in a book and thought I'd share.

[TT(n)]

Thanks for sharing.

I wan't thinking of putting error prone code into the catch block, but you're probably right, if you did. Yet another reason why Try blows hard.
I meant, that with the simple example posted returning would be okay if you closed in the finally.

I just want to make sure that we are not saying that exiting with a return is bad practice. Not at all, that's what it's for, it's new, not old vb6-ish.
Once you know that it means exit with the return value, uhh it's pretty self explanatory, and you shouldn't be using it if you don't know what it does.
Thats why I don't understand "bad practice" logic, or at least it doesn't apply to me, since I would never exit before I had to do something final.
Falling rock behavior is common sense.

[TechGnome]

One of the reasons I advocate Single Entry, Single Exit practice is for maintainability. When there's a problem with a specific part of an application, if it's a function, I tend to go to the Return... breakpoint... find out what it's returning. If there's a single exit, it's easy to find, and I can trace it backwards from there. Plus six months from now when the off-shore developer has to dig through my code, it's consistent. In the end that's what it really is about, reliability and consistency.

I don't necessarily think Try blows... it just requires some creative thinking sometimes to get it to do what you want. So far, I haven't been in a situation where I've thought "Man, I could really use On Error GoTo right now" ... personally, I like Try...Catch ... it allows me to deal with my exceptions right where the happen and I don't need to build error handlers at the bottom, checking for err.number as I go. Having said that... OEGT is nice in that you can Resume Next... with a Try... not so easy... but like I said, it just requires some creative thinking sometimes.

-tg

[TT(n)]

Well I guess if you are using breakpoints, it would be easy to track to.
I just dont' use the debugger at all really, since it causes it's own set of errors. I avoid it completley, and my life is better for it. lol

I only noticed that try blows hard, because I code alot into my library, and most of it is pretty complex. When you get to this level, creativity(of which I have plenty) will get you no where with certain errors.
Plus, when there are several things to catch, the code gets bulky with 3 additional lines per try.

False Premise:
Try, will always continue to try a block of code, when called upon, until it succeeds.

[TechGnome]

False Premise:
Try, will always continue to try a block of code, when called upon, until it succeeds.

Didn't realize that was a premise in the first place, let alone false.

Too bad they couldn't find a way to make ReTry work efficiently. That would have been a truly wonderful asset.

Then again, perhaps is a good thing there isn't a ReTry command, as I could very easily see an infinite loop happening because the database server failed.

[TT(n)]

Just so I'm clear, I don't mean the Try block should be expected to loop itself somehow, but I mean when the block is called upon repeatedly it might ignore the call to even try.

see an infinite loop happening

Yeah, I thought of that too. It might be considered good (unique)for the Try block to lock up, preventing some kind of infinite loop in shotty code. As long as we know it might do that, and we can program accordingly without the expectation that it will fire always.

However, it appears that very few people encounter, or identify it without any documentation about the quirk. Some people may have run across it, but never figured out what was going on.

I think it is just an error, and not "by design", but please correct me if there is documentation on this.

[TechGnome]

I couldn't tell you if there is or not. I've never had a situation where the Catch didn't fire off in the event of an exception. Even in a loop. But then again we're not usually doing large high-speed loops. And more often than not, our loops are inside the Try. 9 times out of 10 they are database writes.

-tg

[dglienna]

Usually, more than one try for the DB. One for the connection, and then one outside the main loop. Nested