0

I have an old app that I'm supporting and I ran across the following Shared SQLConnection:

    private static SqlConnection cnSQL = new SqlConnection(ConfigurationManager.ConnectionStrings["SQL"].ConnectionString.ToString());
    private static SqlCommand cmdSQL = new SqlCommand();

Now this connection is opened and closed everytime it's needed, but it still get a sporatic error, and I believe this is because its being shared accross users (being static). Is my assumption correct? I believe I am much better off creating a new connection inside each method that needs it instead of having one per class. Or can I just remvoe the static option, and keep one connection per page and not have to worry about cross user contamination?

Thanks

Limey
  • 2,642
  • 6
  • 37
  • 62

4 Answers4

3

Get rid of both and declare & define them when you need them.

using (var conn = new SqlConnection(ConfigurationManager.ConnectionStrings["SQL"].ConnectionString.ToString()))
{
    using (var cmd = conn.CreateCommand())
    { ... }
}

Give this answer a read.

Community
  • 1
  • 1
Austin Salonen
  • 49,173
  • 15
  • 109
  • 139
1

The static certainly can make bad things happen if you try to execute two queries at the same time.

Create a new connection in each method where you use it and Dispose at the end would be my preferable architecture. I would also encapsulate the creation of the SqlConnection in a factory method.

Albin Sunnanbo
  • 46,430
  • 8
  • 69
  • 108
1

Static members are shared between all the objects and methods of your code in the actual instance of your application but in no case between different users.

I would make the connection string static but not the connection itself. As you say, open a new connection in each method. Connection pooling will ensure that the physical connections will be kept open.

public static readonly string ConnectionString connString =
    ConfigurationManager.ConnectionStrings["SQL"].ConnectionString.ToString();

...

private void SomeMethod()
{
    using (var conn = new SqlConnection(connString)) {
        string sql = "SELECT ...";
        using (var cmd = new SqlCommand(sql, conn)) {
            ...
        }
    }
}

Make sure you are embedding the code in using-statements. That way Resources are released even when an exception should occur.

Olivier Jacot-Descombes
  • 104,806
  • 13
  • 138
  • 188
  • is a using needed when done in a try with a finally closing the connection? – Limey Nov 12 '12 at 17:54
  • The `using` statement is implemented as a `try-finally` statement and closes the connection for you! I.e. it calls `Dispose()` in the finally part, that in turn closes the connection. This happens in any case, even if you leave the using-statement with a `return` for instance. If you need a try-statement for error handling and are closing the connection withing the `finally` part, of cause no additional `using` is needed. – Olivier Jacot-Descombes Nov 12 '12 at 18:01
0

Sir I'm afraid there is a must that a connection should be initiated just before you call the execute function and closed right after the execution no matter the result is has been failed or not. Being static or not makes no difference if you close and dispose the allocated resources at the right time. The best pattern for initiating a connection is the code below:

SqlCommand command = new SqlConnection("[The connection String goes here]").CreateCommand();

try
{
    command.Parameters.Add(new SqlParameter() { ParameterName = "ParameterA", Direction = ParameterDirection.Input, Value = "Some value" });

    command.Parameters.Add(new SqlParameter() { ParameterName = "ReturnValue", Direction = ParameterDirection.ReturnValue });

    command.CommandText = "[YourSchema].[YourProcedureName]";

    command.CommandType = CommandType.StoredProcedure;

    command.Connection.Open();

    command.ExecuteNonQuery();
}
catch (Exception ex)
{
    //TODO: Log the exception and return the relevant result.
}
finally
{
    if (command.Connection.State != ConnectionState.Closed)

        command.Connection.Close();

    SqlConnection.ClearPool(command.Connection);

    command.Connection.Dispose();

    command.Dispose();
}

Hope it helps.

Cheers

Rikki
  • 3,338
  • 1
  • 22
  • 34
  • Unsure Of why someone down graded, but Should a ClearPool be done? Shouldn't I just leave the connection pool alone and let it deal with itself? – Limey Nov 13 '12 at 14:56
  • I did not downvote, but calling `Dispose()` in the finally part does all what has to be done when closing the connection for you. – Olivier Jacot-Descombes Nov 13 '12 at 15:43
  • @Limey I did check the ClearPool in the profiler. All the documents I've read say that it's not good for the performance of an ODBC connection, because pool keeps the connection like open until another one with the same connection string ask for openning it BUT one time when I was curious about the performance of one of the projects I had 10 month ago, after checking the profiler logs and debugging the code, I conclude the fact that it is better both for SQL (I did check it with 2008 R2) as your database server and your application when you have lots of quick connections like the code above. – Rikki Nov 13 '12 at 17:23
  • @OlivierJacot-Descombes Dear friend, I know that calling the Dispose method will close the connection BUT as a developer I've seen many situations through these eleven years and I've been using this pattern for about ten years and modified it many times, so the reason is that regardless of what Microsoft guys did in the different frameworks (1.* until 4.*) I myself must be sure of the state of the connection, then I can sleep well without nightmares! ;) – Rikki Nov 13 '12 at 17:29
  • By the way thank you guys for paying attention. Wish you all the bests. Cheers – Rikki Nov 13 '12 at 17:30