Code Review: ADO.NET Data Access Utility Class (VB)

Question

When I started working with my current employer, I inherited a project from a previous developer, and this project was equipped with a data access class that tries to simplify a lot of code related to making calls and retrieving data from a database. Over time, it has been modified to add more overloaded forms of functions, and now I am considering possible suggestions from the StackOverflow community.

What do you think needs to be added? Deleted? Modified?

Note:

It would be nice if this class could remain VB.NET compatible in .NET 2.0. We are also developing in 3.5, but I would like to have something that will usually work in most frameworks (so LINQ, etc.). Also, please refrain from unnecessary answers that consist of nothing but "Use nHibernate" or other tools.

My class:

Public Class DataAccess Public Shared Function ReturnScalar(ByVal CmdStr As String) As String Dim Result As String Dim con As SqlConnection = New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As SqlCommand = New SqlCommand(CmdStr, con) cmd.CommandType = CommandType.StoredProcedure Result = cmd.ExecuteScalar con.Close() Return Result End Function Public Shared Function ReturnScalar(ByVal CmdStr As String, ByVal ParamName As String, ByVal Param As Integer) As String Dim Result As String Dim con As SqlConnection = New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As SqlCommand = New SqlCommand(CmdStr, con) cmd.CommandType = CommandType.StoredProcedure cmd.Parameters.Add(New SqlParameter(ParamName, Param)) Result = cmd.ExecuteScalar con.Close() Return Result End Function Public Shared Function ReturnScalar(ByVal CmdStr As String, ByVal ParamName As String, ByVal Param As String) As String Dim Result As String Dim con As SqlConnection = New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As SqlCommand = New SqlCommand(CmdStr, con) cmd.CommandType = CommandType.StoredProcedure cmd.Parameters.Add(New SqlParameter(ParamName, Param)) Result = cmd.ExecuteScalar con.Close() Return Result End Function Public Shared Function ReturnScalar(ByVal CmdStr As String, ByVal Params As Hashtable) As String Dim Result As String Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) cmd.CommandType = CommandType.StoredProcedure For Each entry As DictionaryEntry In Params cmd.Parameters.Add(New SqlParameter(entry.Key.ToString, entry.Value)) Next Result = cmd.ExecuteScalar con.Close() Return Result End Function Public Shared Function ReturnDataSet(ByVal CmdStr As String) As DataSet Dim DS As New DataSet Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) Dim adp As New SqlDataAdapter(cmd) cmd.CommandType = CommandType.StoredProcedure adp.Fill(DS) con.Close() Return DS End Function Public Shared Function ReturnDataSet(ByVal CmdStr As String, ByVal ParamName As String, ByVal Param As Integer) As DataSet Dim DS As New DataSet Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) Dim adp As New SqlDataAdapter(cmd) cmd.CommandType = CommandType.StoredProcedure cmd.Parameters.Add(New SqlParameter(ParamName, Param)) adp.Fill(DS) con.Close() Return DS End Function Public Shared Function ReturnDataSet(ByVal CmdStr As String, ByVal ParamName As String, ByVal Param As String) As DataSet Dim DS As New DataSet Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) Dim adp As New SqlDataAdapter(cmd) cmd.CommandType = CommandType.StoredProcedure cmd.Parameters.Add(New SqlParameter(ParamName, Param)) adp.Fill(DS) con.Close() Return DS End Function Public Shared Function ReturnDataSet(ByVal CmdStr As String, ByVal Params As Hashtable) As DataSet Dim DS As New DataSet Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) Dim adp As New SqlDataAdapter(cmd) cmd.CommandType = CommandType.StoredProcedure For Each entry As DictionaryEntry In Params cmd.Parameters.Add(New SqlParameter(entry.Key.ToString, entry.Value)) Next adp.Fill(DS) con.Close() Return DS End Function Public Shared Function ReturnDataTable(ByVal CmdStr As String) As DataTable Dim DT As New DataTable Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) Dim adp As New SqlDataAdapter(cmd) cmd.CommandType = CommandType.StoredProcedure adp.Fill(DT) con.Close() Return DT End Function Public Shared Function ReturnDataTable(ByVal CmdStr As String, ByVal ParamName As String, ByVal Param As Integer) As DataTable Dim DT As New DataTable Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) Dim adp As New SqlDataAdapter(cmd) cmd.CommandType = CommandType.StoredProcedure cmd.Parameters.Add(New SqlParameter(ParamName, Param)) adp.Fill(DT) con.Close() Return DT End Function Public Shared Function ReturnDataTable(ByVal CmdStr As String, ByVal ParamName As String, ByVal Param As Date) As DataTable Dim DT As New DataTable Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) Dim adp As New SqlDataAdapter(cmd) cmd.CommandType = CommandType.StoredProcedure cmd.Parameters.Add(New SqlParameter(ParamName, Param)) adp.Fill(DT) con.Close() Return DT End Function Public Shared Function ReturnDataTable(ByVal CmdStr As String, ByVal ParamName As String, ByVal Param As String) As DataTable Dim DT As New DataTable Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) Dim adp As New SqlDataAdapter(cmd) cmd.CommandType = CommandType.StoredProcedure cmd.Parameters.Add(New SqlParameter(ParamName, Param)) adp.Fill(DT) con.Close() Return DT End Function Public Shared Function ReturnDataTable(ByVal CmdStr As String, ByVal Params As Hashtable) As DataTable Dim DT As New DataTable Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) Dim adp As New SqlDataAdapter(cmd) cmd.CommandType = CommandType.StoredProcedure For Each entry As DictionaryEntry In Params cmd.Parameters.Add(New SqlParameter(entry.Key.ToString, entry.Value)) Next adp.Fill(DT) con.Close() Return DT End Function Public Shared Function DBExecute(ByVal CmdStr As String) As Integer Dim RowsAffected As Integer Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) cmd.CommandType = CommandType.StoredProcedure RowsAffected = cmd.ExecuteNonQuery con.Close() Return RowsAffected End Function Public Shared Function DBExecute(ByVal CmdStr As String, ByVal ParamName As String, ByVal Param As Integer) As Integer Dim RowsAffected As Integer Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) cmd.CommandType = CommandType.StoredProcedure cmd.Parameters.Add(New SqlParameter(ParamName, Param)) RowsAffected = cmd.ExecuteNonQuery con.Close() Return RowsAffected End Function Public Shared Function DBExecute(ByVal CmdStr As String, ByVal Params As Hashtable) As Integer Dim RowsAffected As Integer Dim con As New SqlConnection(ConfigurationManager.ConnectionStrings("RoutingConnectionString").ToString) con.Open() Dim cmd As New SqlCommand(CmdStr, con) cmd.CommandType = CommandType.StoredProcedure For Each entry As DictionaryEntry In Params cmd.Parameters.Add(New SqlParameter(entry.Key.ToString, entry.Value)) Next RowsAffected = cmd.ExecuteNonQuery con.Close() Return RowsAffected End Function End Class 

Note 2:

"RoutingConnectionString" is simply the name ConnectionString in this particular version of the class and is not something that will remain the same in all of our projects.

Note 3:

All our projects use SQLServer2005, but I don’t see anything wrong with the modifications that could help make it more flexible with other database systems such as MySQL or older / newer versions of SQL Server.

Answer 1

It takes a little time to study the class above. However, a very similar question was asked about a month ago in my discussion group , and you might find it advisable to read the answers.

In the meantime, I will upload my class and analyze it. When done, I will edit my post to include my comments.

---

I hope you have a look at the thread I'm connected with, so I will not list the points mentioned there that are relevant here. There is no point in repeating when it will be a long post anyway !; -)

Problem 1: Building / Destruction: The class has no public constructor, so the initialization logic is repeated in every method called. This makes this class a more static class. I don’t think this is the best design solution for the DataLayer class, which is usually designed to be called from hundreds of classes / modules / libraries / pages at once.

Secondly, it has no logic of choice and no finalizer. The layout template is so well matched that VS actually inserts the template into your code the moment you implement IDisposable . The classes associated with the database must contain dispose .

Problem 2: Exception Handling:

2a) Try-catch: There are no Try-Catch-Finally blocks in the class. Your class will not be able to gracefully handle exceptions and pass useful information to the calling code. If an exception occurs while accessing the database, your connection will remain open. Using - End Using constructs Using - End Using highly recommended, and they are basically an abstraction of the Try-Finally pattern.

2b) Error Logging: Your code must have some sort of logging mechanism, although it can also be implemented by the calling code if exceptions are selected from this class.

Problem 3: Excessive / unnecessary overloads: I really do not see the need to have overloads presented.

3a) Repetition: Most of the code in overloads seems to be repeated. I think this can be handled by a single method with checking for zeros. Or it can be extracted in private methods.

For example:

 Public Shared Function DBExecute(ByVal CmdStr As String, ByVal Params As Hashtable) As Integer ...basic initialization If Params isnot Nothing then For Each entry As DictionaryEntry In Params cmd.Parameters.Add(New SqlParameter(entry.Key.ToString, entry.Value)) Next End If '...rest of processing. End Function 

3b) Prevalence: Secondly, many of your overloads simply result from a modified Parameter data type. To solve this problem, you might find it easier to reflect the base type of the element value in a HashTable. Another way I've used in the past is to have a single method signature and allow the user to pass an array from SqlParameters. This gives the user complete freedom to set properties such as data types, size, direction, accuracy and precision. The third method I'm currently using is to have a settable property of type SqlParameterCollection , which will be set by the end user and evaluated before each type of database method is executed.

Finally, I see no reason to have separate methods (and overloads) for ReturnDataSet and ReturnDataTable , since the former will return a collection of DataTables anyway.

Problem 4: Reusing connections: Using the Shared class template without a constructor means that the user cannot run subsequent requests using the same object without having to go through the same initialization.

Problem 5: Support for stored procedure: CommandType SqlCommand cannot be set to Text . It is assumed that all commands are of type StoredProcedure .

Problem 6: ConnectionStrings:

6a) Encryption: The class does not support encrypted connection string support in the configuration file.

6b) Configurability: You mentioned that the ConnectionString key name may be different. How will this be installed without recompiling the class?

Problem 7: DataAdapter Relationships: When using DataAdapters (for example, in ReturnDataSet methods), it is always better to let the DataAdapter control the opening and closing of the connection.

Problem 8: ReturnScalar Data Types: Your ReturnScalar methods assume a String return type. Firstly, there is no explicit cast (my global parameter "Explicit On" is immediately flagged by this error). Secondly, what of the other data types? I think you should return an Object , and let the calling code be reset as needed.

Problem 9: Xml support: None of the methods allow calling the ExecuteXmlReader method, which is specific to the SqlClient provider. You may not need this in your scenario, but I thought it was worth mentioning.

I think about wraps. Please do not pay attention to this, but if you asked me to rate this class, I would give it 3 to 10. These 3 are for good intentions and for a commendable initiative to encapsulate the logic of accessing the database in a separate library.

Answer 2

A few observations:

1) You can make your execution routines more flexible by using ParamArray as an argument. Thus, you can only have a few methods that can handle the various types of SQL parms and their number. The caller can simply pass in any values ​​needed for this stored procedure, then you can use SqlCommandBuilder.DeriveParameters to populate your SqlCommand object. The caller will need to make sure that they send the parm in the order that the SP expects them, but this should be acceptable.

2) It seems that all SqlCommand executions use a default timeout. There may be times when you need to increase it, therefore, having ReturnDataSet / ReturnDataTable / etc as an optional argument. may be beneficial.

3) You might think of having a separate set of routines that perform the same actions, but within a transaction.