patterncsharpMinor
Dapper helper service
Viewed 0 times
helperservicedapper
Problem
I've written a very light wrapper around Dapper to help with unit testing and streamlining the Dapper commands I need.
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?
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(int id) where T : class
{
return Execute((x, t) => x.Get(id, t));
}
public bool Delete(T dto) where T : class
{
return Execute((x, t) => x.Delete(dto,t));
}
public dynamic Insert(T dto) where T : class
{
return Execute((x, t) => x.Insert(dto,t));
}
public void Insert(List dtoList) where T : class
{
IEnumerable enu = dtoList;
using (var con = new SqlConnection(sqlConnection))
{
con.Open();
var trans = con.BeginTransaction();
con.Insert(enu, trans);
trans.Commit();
con.Close();
}
}
public IEnumerable GetList() where T : class
{
return Execute((x, t) => x.GetList(transaction:t));
}
public dynamic Execute(Func action)
{
using (var con = new SqlConnection(sqlConnection))
{
con.Open();
var trans = con.BeginTransaction();
var result = action(con, trans);
trans.Commit();
con.Close();
return result;
}
}
}Solution
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.
Although for that method
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.
Shows the exact purpose of your lambda much more clearly.
I also note that you have a method called
Lastly, you should avoid putting the type of the variable in its name.
E.g.
would be better as:
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
Parameter types
You take a
And then immediately convert it to an
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 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(List dtoList)would be better as:
public void Insert(List 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(List dtoList) where T : class
{
IEnumerable 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.Code Snippets
public dynamic Insert<T>(T dataTransferObject) where T : class
{
return Execute((x, t) => x.Insert(dataTransferObject,t));
}return Execute((connection, transaction) => connection.Insert(dataTransferObject,transaction));public void Insert<T>(List<T> dtoList)public void Insert<T>(List<T> dtos)public void Insert<T>(List<T> dtoList) where T : class
{
IEnumerable<T> enu = dtoList;
// yadda yadda yadda...Context
StackExchange Code Review Q#69773, answer score: 4
Revisions (0)
No revisions yet.