5
\$\begingroup\$

This is my first time writing async code, I intend to inject the interface IConnection into other classes in my project. Can you please tell me if the implementation class Connection, is the correct usage of await, Task, and async? The code does build, I'm more concerned with async got-ya's that I'm not aware of (I don't know what I don't know). Below is my code, I realise its a bit verbose, but the main part of the code is only about 65 lines.

using System;
using System.Collections.Generic;
using System.Data;
using System.Data.SqlClient;
using System.Linq;
using System.Threading.Tasks;

namespace My.Namespace
{
    /// <summary>Provides both synchronous, and asynchronous methods calls to a database.</summary>
    public interface IConnection
    {
        /// <summary>Maps the result from a stored procedure to a single custom object.</summary>
        /// <typeparam name="T">The type of the custom object that will be mapped from the stored procedure's result.</typeparam>
        /// <param name="reader">A function that takes an IDataReader, and maps the rows / columns to a custom object.</param>
        /// <param name="connectionString">The connectionString to the database.</param>
        /// <param name="usp">The name of the stored procedure.</param>
        /// <param name="parameters">The parameters that the stored procedure expects.</param>
        /// <returns>A custom object with the results from the stored procedure.</returns>
        T ExecuteReaderSingle<T>(Func<IDataReader, T> reader, string connectionString, string usp, params SqlParameter[] parameters);

        /// <summary>Maps the result from a stored procedure to zero or more custom objects.</summary>
        /// <typeparam name="T">The type of the custom object that will be mapped from the stored procedure's result.</typeparam>
        /// <param name="reader">A function that takes an IDataReader, and maps the rows / columns to zero or more custom objects.</param>
        /// <param name="connectionString">The connectionString to the database.</param>
        /// <param name="usp">The name of the stored procedure.</param>
        /// <param name="parameters">The parameters that the stored procedure expects.</param>
        /// <returns>Zero or more custom objects with the results from the stored procedure.</returns>
        IEnumerable<T> ExecuteReader<T>(Func<IDataReader, IEnumerable<T>> reader, string connectionString, string usp, params SqlParameter[] parameters);

        /// <summary>Maps the result from a stored procedure to a single custom object.</summary>
        /// <typeparam name="T">The type of the custom object that will be mapped from the stored procedure's result.</typeparam>
        /// <param name="reader">A function that takes an IDataReader, and maps the rows / columns to a custom object.</param>
        /// <param name="connectionString">The connectionString to the database.</param>
        /// <param name="usp">The name of the stored procedure.</param>
        /// <param name="parameters">The parameters that the stored procedure expects.</param>
        /// <returns>A task of a custom object with the results from the stored procedure.</returns>
        Task<T> ExecuteReaderSingleAsync<T>(Func<IDataReader, T> reader, string connectionString, string usp, params SqlParameter[] parameters);

        /// <summary>Maps the result from a stored procedure to zero or more custom objects.</summary>
        /// <typeparam name="T">The type of the custom object that will be mapped from the stored procedure's result.</typeparam>
        /// <param name="reader">A function that takes an IDataReader, and maps the rows / columns to zero or more custom objects.</param>
        /// <param name="connectionString">The connectionString to the database.</param>
        /// <param name="usp">The name of the stored procedure.</param>
        /// <param name="parameters">The parameters that the stored procedure expects.</param>
        /// <returns>A task of zero or more custom objects with the results from the stored procedure.</returns>
        Task<IEnumerable<T>> ExecuteReaderAsync<T>(Func<IDataReader, IEnumerable<T>> reader, string connectionString, string usp, params SqlParameter[] parameters);

        /// <summary>Executes the stored procedure.</summary>
        /// <param name="connectionString">The connectionString to the database.</param>
        /// <param name="usp">The name of the stored procedure.</param>
        /// <param name="parameters">The parameters that the stored procedure expects.</param>
        /// <returns>The number of rows affected by the stored procedure.</returns>
        int ExecuteNonQuery(string connectionString, string usp, params SqlParameter[] parameters);

        /// <summary>Executes the stored procedure.</summary>
        /// <param name="connectionString">The connectionString to the database.</param>
        /// <param name="usp">The name of the stored procedure.</param>
        /// <param name="parameters">The parameters that the stored procedure expects.</param>
        /// <returns>A task containing the number of rows affected by the stored procedure.</returns>
        Task<int> ExecuteNonQueryAsync(string connectionString, string usp, params SqlParameter[] parameters);
    }

    public class Connection : IConnection
    {
        public T ExecuteReaderSingle<T>(Func<IDataReader, T> reader, string connectionString, string usp, params SqlParameter[] parameters)
        {
            IEnumerable<T> result = ExecuteReader<T>(dr => Enumerable.Repeat(reader(dr), 1), connectionString, usp, parameters);
            return result.First();
        }

        public IEnumerable<T> ExecuteReader<T>(Func<IDataReader, IEnumerable<T>> reader, string connectionString, string usp, params SqlParameter[] parameters)
        {
            Task<T[]> task = ExecuteCommand(async command => reader(await command.ExecuteReaderAsync()).ToArray(), connectionString, usp, parameters);
            return GetResultOf(task);
        }

        public async Task<IEnumerable<T>> ExecuteReaderAsync<T>(Func<IDataReader, IEnumerable<T>> reader, string connectionString, string usp, params SqlParameter[] parameters)
        {
            return await ExecuteCommand(async command => reader(await command.ExecuteReaderAsync()).ToArray(), connectionString, usp, parameters);
        }

        public async Task<T> ExecuteReaderSingleAsync<T>(Func<IDataReader, T> reader, string connectionString, string usp, params SqlParameter[] parameters)
        {
            IEnumerable<T> result = await ExecuteCommand(async command => Enumerable.Repeat(reader(await command.ExecuteReaderAsync()), 1), connectionString, usp, parameters);
            return result.First();
        }

        public int ExecuteNonQuery(string connectionString, string usp, params SqlParameter[] parameters)
        {
            Task<int> task = ExecuteCommand(async command => await command.ExecuteNonQueryAsync(), connectionString, usp, parameters);
            return GetResultOf(task);
        }

        public async Task<int> ExecuteNonQueryAsync(string connectionString, string usp, params SqlParameter[] parameters)
        {
            return await ExecuteCommand(async command => await command.ExecuteNonQueryAsync(), connectionString, usp, parameters);
        }

        /// <summary>
        /// Converts asynchronous calls to synchronous calls, retrieving the result from a task.
        /// If the task resulted in an exception, the exception is thrown instead.
        /// </summary>
        private static T GetResultOf<T>(Task<T> task)
        {
            return task.ContinueWith<TaskResult<T>>(t =>
                {
                    T tResult = t.IsFaulted ? default(T) : t.Result;
                    Exception exception = t.IsFaulted ? t.Exception.InnerExceptions.First() : null;

                    return new TaskResult<T>(tResult, exception);
                }).Result;
        }

        /// <summary>
        /// Executes commands asynchronously to the database.
        /// This method is generic enough to be used for any command.
        /// </summary>
        private static async Task<T> ExecuteCommand<T>(Func<SqlCommand, Task<T>> execute, string connectionString, string usp, params SqlParameter[] parameters)
        {
            using (var connection = new SqlConnection(connectionString))
            {
                using (var command = new SqlCommand(usp, connection))
                {
                    command.CommandType = CommandType.StoredProcedure;
                    command.Parameters.AddRange(parameters);

                    await connection.OpenAsync();
                    return await execute(command);
                }
            }
        }

        /// <summary>
        /// When casted to T, this type will return T as long as there
        /// is no exception specified, otherwise the exception will be thrown.
        /// </summary>
        private struct TaskResult<T>
        {
            private T result;
            private Exception exception;

            public TaskResult(T result, Exception exception)
            {
                this.result = result;
                this.exception = exception;
            }

            public static implicit operator T(TaskResult<T> taskResult)
            {
                if (taskResult.exception != null)
                {
                    throw taskResult.exception;
                }

                return taskResult.result;
            }
        }
    }
}

An example of the usage would be:

using System.Data.SqlClient;

namespace My.Namespace
{
    public class DatabaseSelect
    {
        private readonly IConnection _connection = null;

        public DatabaseSelect(IConnection connection)
        {
            _connection = connection;
        }

        public string GetUserName(int id)
        {
            string username = _connection.ExecuteReaderSingle<string>(dr => dr.Get<string>("UserName"), "connectionString", "storedProcedureName", new SqlParameter("@Id", id));
            return username;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ why u sending dr.Get<string>("UserName") into the fetching code, it should be done only after it \$\endgroup\$ – paritosh Aug 6 '14 at 4:46
  • \$\begingroup\$ @paritosh that datareader statement is called only after the SqlCommand object is ready, its a lazy lambda. \$\endgroup\$ – user1909158 Aug 6 '14 at 5:18
  • \$\begingroup\$ what I meant is that do this inside of the code GetUserName method not when your executing the reader so that it will be out side of data fetching code. so once datareader has been filled read that data \$\endgroup\$ – paritosh Aug 6 '14 at 5:57
  • \$\begingroup\$ @paritosh by that time the SqlConnection is already disposed. \$\endgroup\$ – user1909158 Aug 6 '14 at 6:34
  • \$\begingroup\$ Just FYI, take a look at Rx - Reactive Extensions - "linq for IObserable" \$\endgroup\$ – radarbob Aug 6 '14 at 15:37
1
\$\begingroup\$
private static T GetResultOf<T>(Task<T> task)
{
    return task.ContinueWith<TaskResult<T>>(t =>
        {
            T tResult = t.IsFaulted ? default(T) : t.Result;
            Exception exception = t.IsFaulted ? t.Exception.InnerExceptions.First() : null;

            return new TaskResult<T>(tResult, exception);
        }).Result;
}

How is all this different from just task.Result? That it throws only the first exception? You can achieve that by calling task.GetAwaiter().GetResult().

An advantage of that approach is that it also doesn't reset the exception's stack trace, unlike your TaskResult. (If you want to fox that, look into ExceptionDispatchInfo.)

Also, this is likely to cause deadlocks if called from code with synchronization context set (which includes the UI thread of GUI applications and request threads in ASP.NET). To fix this, you would need to use ConfigureAwait(false) for every await in this code.

But an even better option is not to expose synchronous wrappers for asynchronous methods at all.


return await ExecuteCommand(…)

If you remove both the await and the async modifier from the method, the code will work the same, only with slightly less overhead.


This doesn't really sound like Connection, since each command can be to a different database. I would consider either moving the connectionString parameter to the constructor of Connection, or renaming the class (and interface) to something else.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your feed back, I replaced the await and async with just tasks. The reaosn I do the error handling like that is because I don't want the caller of the sync methods to know its a task under the hood, so I wnat to throw the callers exception, not an AggregateException. I ran this code and it seems to never call the database, any ideas why? Also should I use all the async method on the SqlCommand, or sould I only use one top level async call? \$\endgroup\$ – user1909158 Aug 7 '14 at 1:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.