1

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:

  • The app prints out "1" (user input), "1" (database result), "0" (enum of Closed connection)
  • When I press Enter to get past the first Console.ReadLine(), the app simply closes out.

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

Improve this question
1
  • tsk tsk - string concatenation to add your batchid to the sql query is naughty. Use parameters like you did for your stored procedure instead. Commented Oct 26, 2011 at 15:37

2 Answers 2

Reset to default
2

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
Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

reader.Read() returns false if there are no rows. As mentioned in the Console.WriteLine() line, the data is actually being returned successfully. Executing the SELECT statement in GetBatchRevision() in SQL itself returns one row, so reader.Read() cannot possibly be false here because SqlDataReader.Read starts before the first record.
@Kevin regardless. That piece of code is buggy as hell. You'll get surprises, believe me.
Your suggested alternative makes sense to me, however upon trying it in my code I have discovered that the reader omits the first row returned by the SqlCommand query every time. Why is that?
1

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.

Improve this answer

2 Comments

Thanks for the tips. I originally had a new SqlConnection initiated individually every time I wanted to access the DB. My main motivation for changing that was that the app actually has about 15 different functions so I thought it was bad practice to declare the same connection 15 separate times. With GetBatchRevision, I did have it accept an argument at first. When I realized that the global var batchID worked fine, I tried to see if making it interact with a global batchRev would work around the runtime error (it didn't, sadly). With rev/id, I've an ulterior motive for using String.
Your solution worked, thanks! I really should work on sticking more closely to better coding practices, seeing as the error was probably arising from a hanging connection.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.