Dapper helper service

Question

I've written a very light wrapper around Dapper to help with unit testing and streamlining the Dapper commands I need.

Execute() is a method created to remove the code duplication of opening and closing the database connection and enlisting it in a transaction. The code is very quick at inserts with this method (200,000 inserts in less than 30 secs).

I'm wondering if I've made any obvious mistakes with regards to Dapper and performance.

For instance, should a single insert or read explicitly enlist a transaction, or would they be faster with an ambient transaction?

public class DappperService 
{
    private readonly string sqlConnection;

    public DappperService(string sqlConnection)
    {
        this.sqlConnection = sqlConnection;
        DapperExtensions.DapperExtensions.DefaultMapper = typeof(PluralizedAutoClassMapper<>);
    }

    public T Get<T>(int id) where T : class
    {
        return Execute((x, t) => x.Get<T>(id, t));
    }

    public bool Delete<T>(T dto) where T : class
    {
        return Execute((x, t) => x.Delete(dto,t));
    }

    public dynamic Insert<T>(T dto) where T : class
    {
        return Execute((x, t) => x.Insert(dto,t));
    }

    public void Insert<T>(List<T> dtoList) where T : class
    {
        IEnumerable<T> enu = dtoList;

        using (var con = new SqlConnection(sqlConnection))
        {
            con.Open();
            var trans = con.BeginTransaction();
            con.Insert(enu, trans);
            trans.Commit();
            con.Close();
        }
    }

    public IEnumerable<T> GetList<T>() where T : class
    {
        return Execute((x, t) => x.GetList<T>(transaction:t));
    }

    public dynamic Execute(Func<SqlConnection, SqlTransaction, dynamic> action)
    {
        using (var con = new SqlConnection(sqlConnection))
        {
            con.Open();
            var trans = con.BeginTransaction();
            var result = action(con, trans);
            trans.Commit();
            con.Close();
            return result;
        }
    }
}

Answer 1

Naming

I recommend not using acronyms or shortened names where possible for your variables. You might as well use the full name, it aids in quick reading.

e.g.

public dynamic Insert<T>(T dataTransferObject) where T : class
{
    return Execute((x, t) => x.Insert(dataTransferObject,t));
}

Although for that method item may work just as well.

The same goes for single-letter variable names in your lambda expressions. If there's only parameter to your lambda, it's probably ok, but with multiple parameters it can quickly become confusing.

return Execute((connection, transaction) => connection.Insert(dataTransferObject,transaction));

Shows the exact purpose of your lambda much more clearly.

I also note that you have a method called GetList that does not return a List. This is just plain confusing.

Lastly, you should avoid putting the type of the variable in its name.

E.g.

public void Insert<T>(List<T> dtoList)

would be better as:

public void Insert<T>(List<T> dtos)

Otherwise it can lead to confusion should you decide to change it to a different type, and forget to update the variable name.

var

You have used var appropriately and consistently, making this (as far as I'm aware) the first code review I've ever posted that does not contain some guidance on var. Well done.

Parameter types

You take a List with this method:

public void Insert<T>(List<T> dtoList) where T : class
{
    IEnumerable<T> enu = dtoList;
    // yadda yadda yadda...

And then immediately convert it to an IEnumerable in your body. Just take the parameter as IEnumerable in the first place, passing a List in will still work. By requiring a List you're actually making it harder to use your own function (because you can't, for example, pass in an array) for no reason at all.