HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Simple SqlHelper which wraps ADO.NET methods

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplewrapssqlhelperadomethodsnetwhich

Problem

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 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 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 ExecuteScalarAsync(string connectionString, CommandType cmdType, string cmdText,
params SqlParameter[] commandParameters)
{
using (var connection = new SqlConnection(connectionString))
{
using (var command = new SqlCommand(cmdText, connection))
{

Solution

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.

Code Snippets

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();
        }
    }
finally
    {
        connection.Close();
    }
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();
    }

Context

StackExchange Code Review Q#63480, answer score: 8

Revisions (0)

No revisions yet.