Avoid NullReferenceException

Question

I'm fairly new to .NET programming. I am trying to check if there are settings in a database table and if not, create some default settings. The problem is that when there are no settings I need to make two calls with the datareader and it keeps giving me problems.

I originally was only using one datareader but made two to attempt resolving my issue, didn't work.
Then I tried closing the datareader and I get errors because it returned a nullreference.

If I try to close it I get an issue, if I don't close it the next one gives me an error saying there is already an open datareader on this connection that needs to be closed. Can anyone tell me how I can refactor my code to get this to work (preferably with one datareader)?

I also tried putting the reader.close() in another set of try catch, while that enables me to catch the error, it still doesn't close the datareader so I am at a loss.

Private Sub Get_Initial_Settings()
    Dim reader1 As MySqlDataReader = Nothing, reader2 As MySqlDataReader = Nothing
    Dim cmd As New MySqlCommand("SELECT depot, size, roc_family, wil_family, ast_family, met_family, ric_family, view FROM vb_dashboard.user_preferences WHERE " & GetUserName() & "", conn)
    Dim hasSettings As Boolean

    'Get Personal Settings or initiate them if necessary
    Try
        reader1 = cmd.ExecuteReader
        hasSettings = True
        MessageBox.Show("Your user settings show you have a selected depot of " & reader1(0).ToString)
    Catch ex As Exception
        'No settings exist, set some up
        MessageBox.Show("You have no settings for this program yet")
        hasSettings = False
    Finally
        reader1.Close()
    End Try

    'User has no preferences, Create some
    'First, create a list of depots to select from and add it to a combobox
    If (hasSettings = False) Then
        Try

            cmd.CommandText = "SELECT depot FROM vb_dashboard.depots ORDER BY depot"
            reader2 = cmd.ExecuteReader
            While (reader2.Read)
                dlgSelectDepot.cbDepotSelect.Items.Add(reader2.GetString(0))
            End While
            'Now show the dialog box to initiate a depot setting
            Me.Hide()
            dlgSelectDepot.Show()
            Me.Show()
            If (dlgSelectDepot.DialogResult = Windows.Forms.DialogResult.Cancel) Then
                Me.Close()
            End If
            cmd.CommandText = "INSERT INTO vb_database.user_preferences SET user='" & GetUserName.ToUpper & "', depot='Rochester'"
            cmd.ExecuteNonQuery()
        Catch ex As Exception
            MessageBox.Show("An error has occurred: " & ex.Message)
        Finally
            reader2.Close()
        End Try
    End If

End Sub

IAbstract · Answer 1 · 2010-12-17T19:43:51.207

0

You are re-using your cmd object. Effectively, you are setting each reader to the same connection as your command objects. You should create 2 different command objects that are given their own database connections.

Another option would be to get info you need from the first data reader and store in a datatable (using a MySqlDataAdapter). If that info is not available, close that connection, create a new one with a new command object. This is probably the direction I would go.

Edit

You really need to use New SqlConnection() and let the OS handle that.

edited Dec 17 '10 at 19:43

answered Dec 17 '10 at 19:38

IAbstract

19,551
15
98
146

Thanks for the quick response. So I have to create a new connection for every call to the DB? That is going to be a LOT of connections. Is there any way to maintain one connection and just reuse it? – Geoff Dec 17 '10 at 19:40
You can make several calls on one connection, but I believe it is bad practice as the OS isn't able to manage them. Making connections is not resource intensive. Every call I make is wrapped in a `Using` clause. I can easily have 7 or 8 opening and closing at any given moment. – IAbstract Dec 17 '10 at 19:46
Cool, I thought it would be bad practice to keep making connections with the Using clause. I'd rather use Using and avoid Try/Catch all together. Thanks, I'll work on that – Geoff Dec 17 '10 at 20:05
`Using` clause is your best friend. I have made it a habit to look into a class that I want to use and determine if it implements `IDisposable` and if it does, then I consider first and foremost whether I should use `Using`. – IAbstract Dec 17 '10 at 20:10
I don't think it's necessary or ideal to re-open a connection for every single SQL statement you want to execute. You just need to correctly dispose of objects only (and always) after they have been successfully created. Using will make that easier but you'll have to nest it within a Try/Catch if you also want exception handling whereas Try/Catch/Finally can accomplish it all at one level. – BlueMonkMN Dec 18 '10 at 15:55
@Blue: If I am executing several commands to the same table, I will do so with the same connection object. However, if I am performing any work (transforming DataTable, sorting, further queries on results, or results manipulation) between command execution, I close that connection then do my work. – IAbstract Dec 18 '10 at 16:03
Best then to make sure the poster knows how to do both methods properly rather than implying that the only solution to this problem is to *always* close the connection. – BlueMonkMN Dec 18 '10 at 16:37

score 0 · Answer 2 · answered Dec 17 '10 at 20:03

The problem is in your second try block you are executing a command that still has an open reader. i use Using blocks to help keep them straight, for example:


Using cn As new DbConnection(connectionString)
 cn.Open()
 Using cmd As DbCommand = cn.CreateCommand()
  'first command
  cmd.CommandText = "SELECT depot FROM vb_dashboard.depots ORDER BY depot" 
  Using dr As DbDataReader = cmd.ExecuteReader()
   While dr.Read()
    dlgSelectDepot.cbDepotSelect.Items.Add(reader2.GetString(0))
   End While
   dr.Close()
  End Using
  '
  'next command
  cmd.CommandText = "INSERT INTO vb_database.user_preferences SET user='" & GetUserName.ToUpper & "', depot='Rochester'"
  cmd.ExecuteNonQuery()
 End Using
 cn.Close()
End Using

I think the problem with this solution is there's no exception handling so the code can't detect when the user settings have not been initialized. — BlueMonkMN, Dec 18 '10 at 15:49

score 0 · Answer 3 · answered Dec 18 '10 at 15:47

I think the problem is that you have two potential problem causes and can only solve one at a time with the kind of code you are using. Here are the two possibilities:

No user settings exist yet.
You are re-using the same command and connection objects for multiple queries.

With the code you currently have, I think you have solved problem #2, but re-introduced problem number 1. If no user settings exist yet, the datareader never gets created because there's a failure executing the command. So when you try to close the datareader, you get an error. When the code did not close the datareader, you solved problem #1, but not #2. I think there are a couple better ways to solve this:

Find a better way to detect whether the user settings exist. For example, SELECT COUNT(*) FROM sys.objects where name = 'user_preferences'
Check "If reader1 IsNot Nothing" before trying to close it.

Avoid NullReferenceException

3 Answers3