0

I'm refactoring legacy code and I came accros alot of code like this, and I want to know if I must dispose of IDbParameter:

Private Function GetSqlObject as Object
    Try
        Dim parmeters() As IDbDataParameter = CreateParameters(criterias)

        reqSQL = "A SQL Request"

        dataReader = DBHelper.ExecuteReader(CommandType.Text, reqSQL, parmeters)

        While dataReader.Read()
        ....
        End While

        Return ...
    Finally
        DisposeParameters(parm)


        If Not dataReader Is Nothing Then
            dataReader.Close()
            dataReader.Dispose()
        End If

    End Try
End Sub 
    
Private Sub DisposeParameters(ByVal parameters As IDbDataParameter())
    If parameters Is Nothing Then Return
    Dim dbDataParameterArray As IDbDataParameter() = parameters
    Dim index As Integer = 0

    While index < dbDataParameterArray.Length
        dbDataParameterArray(index).Dispose()
        index += 1
    End While
End Sub

I wonder if I can completly remove the Finally statement just by a using:

Using dataReader = DBHelper.ExecuteReader(CommandType.Text, reqSQL, parmeters)
    While dataReader.Read()
    ....
    End While
End Using

In that case, I know that the dataReader as stated here : correct point to close a data reader in vb.net

But how about the parameters ? Do I need to dispose them ? Is it just safe to let them garbage collected and remove completly the Finally and DisposeParameters function ?

I have a little confusion about reading this article: Proper use of the IDisposable interface

Since the parameters is not a DB connection or such, I think it's managed ? If so, I don't need to dispose it ?

JFPicard
  • 5,029
  • 3
  • 19
  • 43
  • 1
    Do you have a `Dispose()` method to call on a DbParameter? -- You don't need this: `dataReader.Close() dataReader.Dispose()`. Pick one or the other (I suggest the latter, to enforce the `IDisposable` pattern; even though it just calls `Close()`, as many other objects that were created before `IDisposable` came by). The `using` *way* is better (especially if you don't have anything else to do in the `finally` block). – Jimi Feb 24 '21 at 20:42
  • 1
    No the parameters do not need disposing. You do need to dispose the connection, reader, transaction if any, and preferably also the command. Do it with `Using` and remove the `Finally` and all references to `Dispose` or `Close` – Charlieface Feb 24 '21 at 20:53
  • @Jimi in DisposeParameters, they do a loop on all params to dispose them. My question is: is this necessary? – JFPicard Feb 25 '21 at 13:25
  • *Do you have a Dispose() method to call on a DbParameter?* -- `IDbDataParameter` Parameters don't have a public `Dispose()` method, since no unmanaged resources are used. The parameter actual Type is undefined here, only the Interface is shown. Neither the `IDbDataParameter` Interface or the `DbParameter` abstract class define a `Dispose()` method. The derived classes (SqlParameter, OleDbParameter etc.) don't either. So, I don't know where that `Dispose()` method comes from. – Jimi Feb 25 '21 at 19:16
  • @Jimi My IDbParameter is an OracleParameter (Oracle.DataAccess.Client.OracleParameter) and it implements IDispose interface – JFPicard Feb 25 '21 at 19:24
  • 1) That method accepts an array of `IDbDataParameter`, which doesn't define `Dispose()`. 2) [OracleParameter](https://learn.microsoft.com/en-us/dotnet/api/system.data.oracleclient.oracleparameter) doesn't expose it (but the Interface is used here, not the implementer). -- Thus, *I don't know where that Dispose() method comes from*. 3) if a new implementation is provided by that code (code that you're not showing here), then there's probably (possibly) a reason. See what a *Parameter* actually is in that code. – Jimi Feb 25 '21 at 19:31
  • If the library implements it, then just call it (though, as mentioned) I don't see how it can be called when the Interface is used. Unless also the Interface has been redefined (and *shadowed*). – Jimi Feb 25 '21 at 19:36

0 Answers0