How can I cleanly use an existing connection or create/dispose of a new one?

Question

I have some instances where sometimes I want to execute code within the same connection (for using temp tables, etc), but most of the time I want to open and close the connection as soon as possible.

public int BulkInsert<T>(IDataReader dataReader, Dictionary<string, string> columnMappings = null, int timeoutInSeconds = 120)
{
    if (_sqlConnection != null)
    {
        return BulkInsert<T>(dataReader, _sqlConnection, columnMappings, timeoutInSeconds);
    }

    using (var tempConnection = new SqlConnection(_connectionString))
    {
        return BulkInsert<T>(dataReader, tempConnection, columnMappings, timeoutInSeconds);
    }
}

How can I make this code more clean and not two separate calls?

My attempt:

public int BulkInsert<T>(IDataReader dataReader, Dictionary<string, string> columnMappings = null, int timeoutInSeconds = 120)
{
    var rv = 0;
    var conn = _sqlConnection ?? new SqlConnection(_connectionString);
    try {
        rv = BulkInsert<T>(dataReader, conn, columnMappings, timeoutInSeconds);
    } finally {
        if (conn != _sqlConnection)
        {
            conn.Dispose();
        }
    }

    return rv;
}

but I am not really happy with it.

P.S. I wasnt sure if this belonged to stackoverflow or programming but I figured because the use of using that it was pretty c# specific and more of a refactor than just an opinion of style

IMO, the first version is fine and even better than the second one. Given the `using`, the first version ensures you the connection is going to be disposed if an exception happens. Cannot say the same about the second though — Camilo Terevinto, Sep 08 '18 at 16:03
why not: `_sqlConnection = _sqlConnection ?? new SqlConnection(_connectionString);` — Ehsan Sajjad, Sep 08 '18 at 16:03
@CamiloTerevinto thanks I forgot about that, added the try..finally, now I really dont like it — ParoX, Sep 08 '18 at 16:05
@EhsanSajjad Notice that the "existing" connection isn't disposed of, so that wouldn't work — Camilo Terevinto, Sep 08 '18 at 16:05

score 0 · Answer 1 · answered Sep 08 '18 at 16:12

I'd recommend the following pattern

private void myFunc()
{
    // The ensureConnection method does the null check and creates a connection
    bool connectionEstablished;
    SqlConnection connection = ensureConnection(out connectionEstablished);
    try
    {
       // Do some work with the connection.

    }
    finally
    {
       // If the connection was established for this call only, the disposeConnection
       // function carries out disposal (or caching etc).
       disposeConnection(connection, connectionEstablished);
    }
}

Its clear what you are trying to achieve and there is no branching. You could encapsulated the SqlConnection and the bool into a simple struct for convenience to cut out the OUT parameter.

The nice option here is that the function doens't know or care where its connection came from, whether it was cached or persisted from another call, and allows for easy multi-threading (the ensureConnection can lease out a persistent connection or create a new one as required if the persistent connection is already in use).

score 0 · Answer 2 · answered Sep 08 '18 at 16:12

Oh I found this method based on C# conditional using block statement

private SqlConnection CreateTempConnectionIfNeeded()
{
    return _sqlConnection == null ? new SqlConnection(_connectionString) : null;
}

public int BulkInsert<T>(IDataReader dataReader, Dictionary<string, string> columnMappings = null, int timeoutInSeconds = 120)
{
    using (var tempConnection = CreateTempConnectionIfNeeded())
    {
        return BulkInsert<T>(dataReader, tempConnection ?? _sqlConnection, columnMappings, timeoutInSeconds);
    }
}

How can I cleanly use an existing connection or create/dispose of a new one?

2 Answers2