ODP.NET Code Example Critque or best practices

Question

I currently have a DataAccess Layer in Vb.Net. I am not too happy with my implementation of both my ExecuteQuery (as DataSet) and ExecuteNonQuery functions. Does anyone have any code that I could see? My code just doesn't look clean. Any thoughts or critiques on it would be appreciated also.

Using odpConn As OracleConnection = New OracleConnection(_myConnString)
                odpConn.Open()
                If _beginTransaction Then

                    txn = odpConn.BeginTransaction(IsolationLevel.Serializable)
                End If
                Try

                    Using odpCmd As OracleCommand = odpConn.CreateCommand()

                        odpCmd.CommandType = CommandType.Text
                        odpCmd.CommandText = sSql
                        For i = 0 To parameters.Parameters.Count - 1
                            Dim prm As New OracleParameter
                            prm = DirectCast(parameters.Parameters(i), ICloneable).Clone
                            odpCmd.Parameters.Add(prm)
                        Next

                        If (odpConn.State = ConnectionState.Closed) Then
                            odpConn.Open()
                        End If
                        iToReturn = odpCmd.ExecuteNonQuery()
                        If _beginTransaction Then
                            txn.Commit()
                        End If
                    End Using

                Catch
                    txn.Rollback()
                End Try

            End Using

Answer 1

A few things

• I would definitly move out the command creation code to its own method. You are probably reusing it anyway, so avoid the copy & pasting.
• You shouldn't need to use using around the command as you are using it on the whole connection anyway. (no harm in doing so though)
• A one query transaction? What is the use in that?
• You open the connection twice.

Here is some code I am using.

 Public Function QueryDataTable(ByVal storedProcedureName As String, ByVal ParamArray parameters As IDbDataParameter()) As DataTable
        Try
            Dim command As SqlCommand = CreateCommand(connection, storedProcedureName, parameters)
            Dim adapter As System.Data.SqlClient.SqlDataAdapter = New System.Data.SqlClient.SqlDataAdapter(command)
            Dim dtResult As New DataTable()

        If transaction Is Nothing Then connection.Open()
        adapter.Fill(dtResult)
        Return dtResult
    Catch ex As Exception
        Log.Append("SQL", "QueryDataTable", ex)
        Throw
    Finally
        If transaction Is Nothing Then connection.Close()
    End Try
End Function

The main things in the above code

• I have broken out the creation of the command to a CreateCommand method.
• As I am implementing transactions on the instance level, the connection object also has to be on an instance level, so I can't use using but instead have to go with to try-finally to ensure that the connection gets closed (when no transaction is running).
• For transactions I implement IDisposable and maintain an open connection during the whole transaction. An alternative to manually implementing transactions would be to make use of the System.Transactions namespace, depending on which database you are targetting, the support it has, and the level of transaction control you need.

Anyway, the above makes it easy to work both with and without transactions. A simple transaction from above would look something like this:

Using sql = new MySqlWrapper(transactionLevel)
   dim dt as DataTable = sql.QueryDataTable(a,b)
   if EverythingOK(dt) then sql.CommitTransaction()
End Using

And when no transactions are needed, I can simply do

dim dt as DataTable = new MySqlWrapper().sql.QueryDataTable(a,b)