Variable 'reader' is used before it has been assigned a value. A null reference exception could result at run time

Question

Trying to fix a warning and not sure how to restructure code as reader.IsClosed is throwing a warning that states "Variable 'reader' is used before it has been assigned a value. A null reference exception could result at run time." Logistically, since reader As SqlDataReader && reader is not initialized with a value then I could ignore as should be fine at runtime, but my inexperience would make me believe there is a better way?

Public Function GetTotalItems(ByVal userId As Long) As Int16

    Dim lstParam As List(Of SqlParameter) = New List(Of SqlParameter)()
    Dim tablMd = Me.GetMetaData()
    Dim retList As ArrayList = New ArrayList()

    lstParam.Add(New SqlClient.SqlParameter("@" + tablMd.PrimaryKey.ColumnName, 0))
    lstParam.Add(New SqlClient.SqlParameter("@UserID", userId))
    lstParam.Add(New SqlClient.SqlParameter("@ActionFlag", "SELECT_ITEMS_COUNT"))

    Dim spName As String = Me.GetStoreProcname()
    Dim reader As SqlDataReader
    Try
        reader = SqlHelper.ExecuteReader(
            Utility.GetConnectionStringSetting(),
            CommandType.StoredProcedure,
            Me.GetStoreProcname(),
            lstParam.ToArray()
        )

        If (reader.HasRows = True) Then
            If (reader.Read()) Then
                Dim value As Object = reader(0)

                Return CInt(value)

            End If

        End If

    Catch ex As Exception
        Throw

    Finally
        If Not reader.IsClosed Then
            reader.Close()
        End If
    End Try

    Return 0

End Function

Hint: `Dim reader As SqlDataReader` needs to be modified, DIY for you. Also you better start using `using` — boop_the_snoot, Feb 01 '18 at 04:01
To be more specific, you are assuming that `reader` has a value when you get its `IsClosed` property in your `Finally` block but if your `SqlHelper.ExecuteReader` method throws an exception then `reader` will not have a value at that point. — jmcilhinney, Feb 01 '18 at 04:16
Also, your `Catch` block is totally pointless. You don't need a `Catch` block if you have a `Finally` block so don't add one if you're not going to do anything in it. As suggested though, you should be using a `Using` block. If you do that then you can do away with the whole `Try...Catch...Finally` because it will achieve the same thing without the possibility of a `NullReferenceException`. — jmcilhinney, Feb 01 '18 at 04:16
Adding to what @jmcilhinney briefly explained, keep [Options Strict & Explicit ON](https://stackoverflow.com/questions/2454552/what-do-option-strict-and-option-explicit-do). — boop_the_snoot, Feb 01 '18 at 04:17
If you create your reader in a `Using` block you won't need to close it in a finally block, plus you will eliminate the warning. — Chris Dunaway, Feb 01 '18 at 14:34

score 0 · Answer 1 · answered Feb 02 '18 at 02:54

@jmcilhinney, @o_O, @Chris Dunaway ... Thank you for the help + appreciation + admiration for your knowledge + reverence == deverence(); ... This removed the error:

Public Function GetTotalAmount(ByVal userId As Long) As Decimal

    Dim lstParam As List(Of SqlParameter) = New List(Of SqlParameter)()
    Dim tablMd = Me.GetMetaData()
    Dim retList As ArrayList = New ArrayList()

    lstParam.Add(New SqlClient.SqlParameter("@" + tablMd.PrimaryKey.ColumnName, 0))
    lstParam.Add(New SqlClient.SqlParameter("@UserID", userId))
    lstParam.Add(New SqlClient.SqlParameter("@ActionFlag", "SELECT_TOTAL_AMOUNT"))

    Dim spName As String = Me.GetStoreProcname()


    Using reader As SqlDataReader = SqlHelper.ExecuteReader(
            Utility.GetConnectionStringSetting(),
            CommandType.StoredProcedure,
            Me.GetStoreProcname(),
            lstParam.ToArray()
        )

        If (reader.HasRows = True) Then
                If (reader.Read()) Then
                    Dim value As Object = reader(0)

                    Return CDec(value)
                End If
            End If
    End Using

    Return 0

End Function

Joel Coehoorn · Accepted Answer · 2018-02-02T20:58:12.673

We can narrow the problem down to this excerpt:

Dim reader As SqlDataReader
Try
    reader = SqlHelper.ExecuteReader( ... )
Finally
    If Not reader.IsClosed Then reader.Close()
End Try

The problem comes if an exception is thrown by the ExecuteReader() function. In that event, the reader variable is never assigned a value. It's still Nothing when you try to evaluate reader.IsClosed, and that will cause an exception.

Given you don't actually do anything with the exception and the SqlHelper takes care of the connection and command objects, you can narrow the entire function down to just this:

Public Function GetTotalItems(ByVal userId As Long) As Int16
    Dim lstParam = {
       New SqlClient.SqlParameter("@" + Me.GetMetaData().PrimaryKey.ColumnName, 0),
       New SqlClient.SqlParameter("@UserID", userId),
       New SqlClient.SqlParameter("@ActionFlag", "SELECT_ITEMS_COUNT")
    }

    Using reader As SqlDataReader = SqlHelper.ExecuteReader(
            Utility.GetConnectionStringSetting(),
            CommandType.StoredProcedure,
            Me.GetStoreProcname(),
            lstParam)

        If reader.Read() Then Return CInt(reader(0))
    End Using

    Return 0  
End Function

So the (reader.HasRows = True) is implicit and is checked before reading so the nested If statements are not required? ... Thanks so much for the help. — McCarson, Feb 02 '18 at 20:53

Variable 'reader' is used before it has been assigned a value. A null reference exception could result at run time

2 Answers2