Why isn't "CreateCommand()" part of C# (or at least .NET)?

Question

After successfully going through the initial stages of learning C# in tandem with SQL Server, I discovered that the various tutorials that I used simply got it wrong by declaring a global SqlConnection, SqlDataAdapter and even DataSet variables.

As a result, this code that works great in a single threaded application, doesn't work so great in a multi-threaded environment. In my research for a solution, I discovered that both MSDN and this educational answer recommend wrapping the "atomic" parts of a SQL transaction in a using/try method:

private static void CreateCommand(string queryString, string connectionString)
{
    using (SqlConnection connection = new SqlConnection(connectionString))
    {
        try
        {
            SqlCommand command = new SqlCommand(queryString, connection);
            command.Connection.Open();
            command.ExecuteNonQuery();
        }
        catch (InvalidOperationException)
        {
            //log and/or rethrow or ignore
        }
        catch (SqlException)
        {
            //log and/or rethrow or ignore
        }
        catch (ArgumentException)
        {
            //log and/or rethrow or ignore
        }
    }
}

So, what I am going to do now is convert my entire code to using wrappers like this. But before proceeding ahead with this I would like to understand the tradeoffs of this approach. In my experience, there usually is a good reason for a large team of designers/engineers for deciding not to include certain defensive features. This is especially interesting when, from my point of view as a C/C++ programmer, the entire value proposition of C# is "defensiveness" (where the tradeoff is the well known CLR performance hit).

To summarize my question(s):

1. What are the tradeoffs of encapsulating every transaction in my code as described above?
2. Are there any caveats I should be looking for?

Answer 1

The reason's down to flexibility. Does the developer want to include the command in a transaction, do they want to retry on a given error, if so how many times, do they want a connection from a thread pool or to create a new connection each time (with a performance overhead), do they want a SQL connection or a more generic DbConnection, etc.

However, MS have provided the Enterprise Library, a suite of functionality which wraps up a lot of common approaches to things in an open source library. Take a look at the Data Access block: http://msdn.microsoft.com/en-us/library/ff632023.aspx

Answer 2

There is no such method built in because:

• Connecting and disconnecting the database for each command is not economical. If you execute more than one command at a given point in the code, you want to use the same connection for them instead of repeatedly opening and closing the connection.

• The method can't know what you want to do about each kind of exception, so the only thing that it could do would be to rethrow them, and then there is no point in catching the exceptions in the first place.

So, almost everything that the method does would be specific for each situatuon.

Besides, the method would have to do more to be generally useful. It would have to take parameters for command type and parameters. Otherwise it can only be used for text queries, and would encourage people to create SQL queries dynamically instead of using stored procedures and/or parameterised queries, and that it not something that a general library would want to do.

Answer 3

1 - There are no real tradeoffs, it's pretty standard.

2 - Your code is ok to send commands as strings to be executed as SQL queries, but it lacks quit a bit of flexibility:

• You can't use parameterized queries (command.Parameters.AddWithValue(...)) which will be mandatory once you start using stored procedures
• You can't use output parameters like this
• You can't do anything with whatever would be queried

I prefer to use something like this:

private static void CallProc(string storedProcName, Action<SqlCommand> fillParams, Action postAction, Action onError)
{
    using (SqlConnection connection = new SqlConnection(connectionString))
    {
        using (SqlCommand command = new SqlCommand(String.Format("[dbo].[{0}]", storedProcName), connection))
        {
            try 
            {
                if(fillParams != null)
                    fillParams(command);
                command.Connection.Open();
                command.ExecuteNonQuery();
                if(postAction != null)
                    postAction();
             }        
            catch (InvalidOperationException)
            {
                //log and/or rethrow or ignore
                throw;
            }
            catch (SqlException)
            {
                //log and/or rethrow or ignore
                throw;
            }
            catch (ArgumentException)
            {
                //log and/or rethrow or ignore
                throw;
            }
            catch
            {
                if(onError != null)
                   onError();
            }
        }
    }
}

You can then make variants to handle return values, output parameters, etc.

And you call is like:

CallProc("myStoredProc",
    command => 
    {
        command.Parameters.AddWithValue("@paramNameOne", "its value here");
        // More parameters for the stored proc...
    },
    null,
    null);

Answer 4

As long as you encapsulate the functionality in a "bottleneck" method like the static method you've posted, so that all your database accesses are implemented in one easy-to-change piece of shared code, there often needs to be no trade-off, because you can change the implementation later without having to rewrite vast tracts of code.

By creating a new connection every time, the risk is that you might incur an expensive overhead for every open/close of the connection. However, the connections should be pooled, in which case the overheads may not be very large and this performance hit may be minimal.

The other approach would be to create a single connection and hold it open, sharing it for all your queries. This is undoubtedly more efficient because you're minimising the overheads per transaction. However, the performance gain may be minimal.

In both cases there will be additional threading (multiple simultaneous queries) issues to resolve unless you make sure that all database queries operate on a single thread. The performance implications all depend on how many queries you're firing off per second - and of course it doesn't matter how efficient your connection approach is if you are using grossly inefficient queries; you need to focus your "optimisation" time on the worst performance issues.

So I'd suggest keeping it simple for now and avoiding premature optimisation, but try to keep the implementation of the database access code in a separate layer, so that your main codebase simply issues commands to the access layer, and has minimal database-specific code in it. The less it "knows" about the database the better. This will make it much easier to change the underlying implementation or port your code to use a different database engine in future.

Another approach that can help with this is to encapsulate queries in stored procedures. This means your program knows the name of the procedure and the parameters for it, but the actual tables/columns that are accessed are hidden inside the database. Your code then knows as little as possible of the low-level structure of the database, which improves its flexibility, maintainability, and portability. Stored procedure calls can also be more efficient than sending generic SQL commands.