Kevin
Kevin

Reputation: 413

SqlDataReader - Multiple connections

I've written a console app in VB.NET to do some database work and a strange runtime error has arisen...

Here's the main code:

Sub Main(ByVal args() As String)
        Try
            user = args(0)
            batchID = args(1)

            GetBatchRevision()
            'batchRev = 1

            Dim getTestScripts As SqlCommand = New SqlCommand("GetTestScriptsInTestBatch", cs)
            getTestScripts.CommandType = CommandType.StoredProcedure

            Dim batchIDParam As SqlParameter = getTestScripts.Parameters.Add("@batchID", SqlDbType.Int, 4)
            Dim batchRevParam As SqlParameter = getTestScripts.Parameters.Add("@batchRev", SqlDbType.Int, 4)

            'batchIDParam.Value = 1
            'batchRevParam.Value = 1
            batchIDParam.Value = batchID
            batchRevParam.Value = batchRev

            Console.WriteLine(batchID & " " & batchRev)
            Console.WriteLine(cs.State)
            Console.ReadLine()

            Using cs
                cs.Open()
                Dim reader As SqlDataReader = getTestScripts.ExecuteReader(CommandBehavior.CloseConnection)

                While reader.Read()
                    Console.WriteLine("Executing Test Script " & reader("ScriptID").ToString() & " Revision " & reader("ScriptRev").ToString)
                End While

                Console.ReadLine()
            End Using

        Catch ex As Exception
        End Try
    End Sub

GetBatchRevision:

Private Sub GetBatchRevision()
    Using cs
        Dim GetNewestRev As New SqlCommand("SELECT Max(BatchRev) FROM TestBatch WHERE BatchID=" & batchID, cs)
        cs.Open()
        Dim reader As SqlDataReader = GetNewestRev.ExecuteReader(CommandBehavior.CloseConnection)
        reader.Read()

        If Not IsDBNull(reader(0)) Then
            batchRev = reader(0).ToString()
        End If
    End Using

End Sub

batchRev and batchID are both global variables within the module.

Behaviorally:

If I comment out GetBatchRevision and directly set batchRev = 1, I get the above result as well as "Executing Test Script 1 Revision 52", "Executing Test Script 2 Revision 66" which are the expected results from the stored procedure GetTestScriptsInTestBatch.

The global variable declarations are as follows:

Private batchID As String

Private batchRev As String

Any ideas why GetBatchRevision() causes the app to crash? By itself (removing the stored proc part of the code), it executes just fine. My initial guess was that there was a hanging connection, but ending a "using" block is supposed to close a SQL connection as well as any open readers associated with said connection (as mentioned before, cs.State returns 0).

Upvotes: 1

Views: 403

Answers (2)

Joel Coehoorn
Joel Coehoorn

Reputation: 416179

It looks like cs is also a global variable. This is a bad idea. .Net data access works a lot better when you're using a new connection each time. You're probably fine in this app, but you're setting up some bad habits. Instead, load your connection string as a global variable and use that when creating your connections.

Next up, there's no reason for GetBatchRevision() to talk to global variables. Have it accept an argument and return it's result instead. And of course I can't overlook the sql injection issue because you concatentate the batchid to the end of your string. Here's the new version of the function after fixing those errors:

Private Function GetBatchRevision(ByVal BatchID As String) As String
    Using cn As New SqlConnection(cs), _
          GetNewestRev As New SqlCommand("SELECT Max(BatchRev) FROM TestBatch WHERE BatchID= @BatchID", cn)

        GetNewestRev.Parameters.Add("@Batch", SqlDbType.Int).Value = Convert.ToInt32(BatchId)

        cn.Open()
        Return GetNewestRev.ExecuteScalar().ToString() 
    End Using

End Function

This can get even better if you keep BatchRev and BatchID as int's rather than strings internally.

Upvotes: 1

Icarus
Icarus

Reputation: 63970

Your problem is on these lines:

 reader.Read()

 If Not IsDBNull(reader(0)) Then

reader.Read() is probably returning false; yet you try to access reader(0). Boom!

You should change it to:

IF reader.Read() AndAlso Not IsDBNull(reader(0)) Then
    '' etc
End If

Upvotes: 2

Related Questions