Simple SqlHelper which wraps ADO.NET methods

Question

I am creating a simple SqlHelper which will simplify the ADO.NET method usage. Can someone please review this class for any issue or missing something?

public static class SqlHelper
{
 public static async Task<int> ExecuteNonQueryAsync(string connectionString, CommandType cmdType, string cmdText,
 params SqlParameter[] commandParameters)
 {
 using (var connection = new SqlConnection(connectionString))
 {
 using (var command = new SqlCommand(cmdText, connection))
 {
 try
 {
 command.CommandType = cmdType;
 command.Parameters.AddRange(commandParameters);
 connection.Open();
 return await command.ExecuteNonQueryAsync();
 }
 finally
 {
 connection.Close();
 }
 }
 }
 }
 public static async Task<SqlDataReader> ExecuteReaderAsync(string connectionString, CommandType cmdType,
 string cmdText, params SqlParameter[] commandParameters)
 {
 using (var connection = new SqlConnection(connectionString))
 {
 using (var command = new SqlCommand(cmdText, connection))
 {
 try
 {
 command.CommandType = cmdType;
 command.Parameters.AddRange(commandParameters);
 connection.Open();
 return await command.ExecuteReaderAsync();
 }
 finally
 {
 connection.Close();
 }
 }
 }
 }
 public static async Task<object> ExecuteScalarAsync(string connectionString, CommandType cmdType, string cmdText,
 params SqlParameter[] commandParameters)
 {
 using (var connection = new SqlConnection(connectionString))
 {
 using (var command = new SqlCommand(cmdText, connection))
 {
 try
 {
 command.CommandType = cmdType;
 command.Parameters.AddRange(commandParameters);
 connection.Open();
 return await command.ExecuteScalarAsync();
 }
 finally
 {
 connection.Close();
 }
 }
 }
 }
}

• Mathieu Guindon: One usability issue I can see is that the client code still needs boilerplate setup code to create the parameters.. OTOH I like that you avoid calling AddWithValue by passing in SqlParameter objects... but "Helper" smells...
• Imran Qadir Baksh - Baloch: @Mat'sMug, Thanks. Your example will be appreciated

Answer 1

Nesting.

You can reduce nesting by stacking the using blocks, like this:

 using (var connection = new SqlConnection(connectionString))
 using (var command = new SqlCommand(cmdText, connection))
 {
 try
 {
 command.CommandType = cmdType;
 command.Parameters.AddRange(commandParameters);
 connection.Open();
 return await command.ExecuteNonQueryAsync();
 }
 finally
 {
 connection.Close();
 }
 }

---

I like your usage of var, and the fact that you've wrapped the IDisposable stuff in using blocks. However this is overkill:

 finally
 {
 connection.Close();
 }

Because the connection is wrapped in a using block, the connection will be cleanly closed and disposed, whatever the reason is for the code to reach the end of the using scope. In other words, you can also ditch the try block, and further reduce nesting:

 using (var connection = new SqlConnection(connectionString))
 using (var command = new SqlCommand(cmdText, connection))
 {
 command.CommandType = cmdType;
 command.Parameters.AddRange(commandParameters);
 connection.Open();
 return await command.ExecuteNonQueryAsync();
 }

That's because a using block is a language shortcut for... a try...finally block.

---

The XxxxxHelper Syndrome

The problem with a static class called SqlHelper, is that over time, it becomes a dumping ground for anything remotely related to SQL; it grows hair and tentacles, and becomes a huge mess before you even notice: "Helper" is simply too vague of a name to result in anything cleanly focused.

I would suggest renaming it to something like SqlCommandWrapper - then it's far less likely to become bloated with things that have nothing to do with executing a SqlCommand, but might be remotely related to SqlWhatever.

• Imran Qadir Baksh - Baloch: Very good explanation. Another I notice is that I should use OpenAsyc. Found in this sample github.com/tjanczuk/edge-sql/blob/master/src/edge-sql/… What do you think?

Answer 2

Having written this type of class before, I would suggest an additional overload with the following signature:

public static async Task<IEnumerable<T>> ExecuteReaderAsync(
 string connectionString, 
 CommandType cmdType,
 string cmdText,
 Func<IDataReader, T> transform), 
 params SqlParameter[] commandParameters)

With this overload, you will iterate over the results returned by the data reader, and for each row, invoke the transform method and store it in a List<T>. When it's finished, return the result to the caller as an IEnumerable<T>.

My particular implementation always returned an empty enumerable, never null. You may choose to do otherwise. (I preferred predictability, and guaranteed that the method never returned null.)

This prevents callers from having to write this type of boilerplate themselves. They simply call:

var foo = await SqlHelper.ExecuteReaderAsync(
 myConnectionString,
 CommandType.StoredProcedure,
 "GetTheFoos",
 r => new Foo { Id = r["foo"], Name = r["Name"] },
 ); // No parameters

Other than that, the other reviews are splendid.

• svick: Why are you suggesting storing the items in a List, instead of yield returning them immediately?
• Mike Hofer: You could do it either way. Your suggestion is undoubtedly better. (+1 for it.) When I wrote my class, I had simply returned the list directly; but there's certainly no reason not to do it that I can think of. Aside, of course, from the fact that in most cases, users will invoke .ToList() on the result to prevent multiple enumeration anyway. (At least, they did at the company where I worked.)

Answer 3

I don't find having to repeat the connection string with each command simpler. Why not make SqlHelper a non-static class, pass the connection string into a constructor and then use it from your methods?

---

Similarly, I believe most commands are going to use CommandType.Text, so that should be the default. You can't use optional parameters together with params, so you'll have to use a separate overload for that.

---

I'm not sure ExecuteReaderAsync is going to work. Doesn't the returned reader need the connection to be open?

• Imran Qadir Baksh - Baloch: @Mat'sMug and svick, got some code from there, github.com/tjanczuk/edge-sql/blob/master/src/edge-sql/…