Opinion on my Data Access Class - VB.NET

[kirby.matt]

Hi, I was wondering if some other experienced .NET developers could
give me opinions, feedback, or critisizms on the data access class I
developed for an application I am starting. I just want to make sure I
am not overlooking anything. I also want to know if I am following
standard conventions. Thanks.

Imports System
Imports System.Data
Imports System.Data.SqlClient
Imports System.Configuration

Namespace DataServices

Public Class DBObject
Private connectionString As String
Private connection As SqlConnection
Private command As SqlCommand
Private parameters As ArrayList

Public Sub New()
connectionString =
ConfigurationSettings.AppSettings("SqlConnString")
connection = New SqlConnection(connectionString)
parameters = New ArrayList()
End Sub

Private Function BuildCommand(ByVal storedProcName As String)
Dim myCommand As New SqlCommand()
Dim param As New SqlParameter()
Try
myCommand.Connection = connection
myCommand.CommandType = CommandType.StoredProcedure
myCommand.CommandText = storedProcName

For Each param In parameters
myCommand.Parameters.Add(param)
Next

Return myCommand
Catch ex As Exception
Throw ex
End Try
End Function

Private Sub BuildParameter(ByVal paramName As String, ByVal
paramType As SqlDbType, ByVal direction As ParameterDirection, ByVal
size As Integer, Optional ByVal value As String = "")
Dim param As New SqlParameter()
Try
If Not paramName.StartsWith("@") Then
paramName = paramName.Concat("@", paramName)
End If
param.ParameterName = paramName
param.DbType = paramType
param.Size = size
param.Direction = direction

If direction = ParameterDirection.Input Then
param.Value = value
End If

parameters.Add(param)
Catch ex As Exception
Throw ex
End Try
End Sub

Public Sub SetInParameter(ByVal paramName As String, ByVal
paramType As SqlDbType, ByVal size As Integer, ByVal value As Object)
Try
Me.BuildParameter(paramName, paramType,
ParameterDirection.Input, size, value)
Catch ex As Exception
If connection.State = ConnectionState.Open Then
connection.Close()
End If
Throw ex
End Try
End Sub

Public Sub SetOutParameter(ByVal paramName As String, ByVal
paramType As SqlDbType, ByVal size As Integer)
Try
Me.BuildParameter(paramName, paramType,
ParameterDirection.Output, size)
Catch ex As Exception
If connection.State = ConnectionState.Open Then
connection.Close()
End If
Throw ex
End Try
End Sub

Public Function ExecuteNonQuery(ByVal storedProcName As String)
As Integer
Dim result As String
Try
Me.connection.Open()
Me.command = Me.BuildCommand(storedProcName)
result = Me.command.ExecuteNonQuery()
Return result
Catch ex As Exception
Throw ex
Finally
If connection.State = ConnectionState.Open Then
connection.Close()
End If
End Try
End Function

Public Function ExecuteReader(ByVal storedProcName As String)
As SqlDataReader
Dim reader As SqlDataReader
Try
Me.connection.Open()
Me.command = Me.BuildCommand(storedProcName)
reader =
Me.command.ExecuteReader(CommandBehavior.CloseConnection)
Return reader
Catch ex As Exception
Throw ex
Finally
If connection.State = ConnectionState.Open Then
connection.Close()
End If
End Try
End Function

Public Function GetDataSet(ByVal storedProcName As String,
ByVal tableName As String) As DataSet
Dim ds As New DataSet()
Dim adapter As New SqlDataAdapter()
Try
Me.connection.Open()
Me.command = Me.BuildCommand(storedProcName)
adapter.SelectCommand = Me.command
adapter.Fill(ds, tableName)
Return ds
Catch ex As Exception
Throw ex
Finally
If connection.State = ConnectionState.Open Then
connection.Close()
End If
End Try
End Function

Public Function GetDataSet(ByVal storedProcName As String,
ByVal ds As DataSet, ByVal tableName As String) As DataSet
Dim adapter As New SqlDataAdapter()
Try
Me.connection.Open()
Me.command = Me.BuildCommand(storedProcName)
adapter.SelectCommand = Me.command
adapter.Fill(ds, tableName)
Return ds
Catch ex As Exception
Throw ex
Finally
If connection.State = ConnectionState.Open Then
connection.Close()
End If
End Try
End Function

Public Function ExecuteScalar(ByVal storedProcName As String)
As Object
Dim result As Object
Try
connection.Open()
Me.command = Me.BuildCommand(storedProcName)
result = Me.command.ExecuteScalar()
Catch ex As Exception
Throw ex
Finally
If connection.State = ConnectionState.Open Then
connection.Close()
End If
End Try

Return result
End Function

Public Sub ClearParameters()
Me.command.CommandText = ""
Me.parameters.Clear()
Me.command.Parameters.Clear()
End Sub
End Class
End Namespace

 

[David Browne]

Hi, I was wondering if some other experienced .NET developers could
give me opinions, feedback, or critisizms on the data access class I
developed for an application I am starting. I just want to make sure I
am not overlooking anything. I also want to know if I am following
standard conventions. Thanks.

Zeroth, think about the code consuming this class and simplfy the client
interface by making most of these methods private. The current design is
hard to use and confusing. See the Enterprise Library Data Block for a good
example of how to do what you're trying to do.

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnpag2/html/entlib.asp

http://msdn.microsoft.com/practices/

First, this class is (or should be) stateless w.r.t the SQLConnection, so
you shouldn't have a SqlConnection instance variable. Just have the
connectionString. Remove the connection, command and parameters instance
variable and replace them with local variables in the various methods.
Currently you are using the instance variables just to pass values from one
method to another without having to define method parameters. That is a bad
thing to do. All instance variables should be initialized in the
constructor. If they are not (as here) it's a big red flag that those
instance variables should not be there.

Second, the structure of your Try/Catch blocks is wrong.


So you have

Try
Me.connection.Open()
Me.command = Me.BuildCommand(storedProcName)
result = Me.command.ExecuteNonQuery()
Return result
Catch ex As Exception
Throw ex
Finally
If connection.State = ConnectionState.Open Then
connection.Close()
End

Which should be

dim Connection as new SqlConnection(connectionString);
connection.Open()
Try
dim command as SqlCommand =
Me.BuildCommand(storedProcName,connection)
result = command.ExecuteNonQuery()
Return result
Finally
connection.Close()
End Try

You should acquire any disposable resources on the line just before Try, not
inside the Ty block. And don't catch the exception if you don't intend to
do anything about it. And when you rethrow just use "Throw" not "Throw ex",
or you'll destroy the stack trace. And upgrade to VB.NET 2.0 so you can use
the Using block.

 

[Cor Ligthert [MVP]]

Just as a little addition to David,

I would set the Connection and the Command even in a nested Try block so
that the Connection errors are as well catched.

Cor

 

[kirby.matt]

Thanks for the tips...good advice. I think I somehow knew that the way
I was doing things was not conventinoal, that's why I decided to post
my code. One question I do have is, how can I have a mechansim for
adding SQLParameters in this class without having the the ArrayList of
SQLParameters as an instance variable? I am trying to create it so the
consumer of this class doesn't have to create SQLParameters objects
outside of this class and then pass them in...any thoughts?