0

I'm trying to write a wrapper to hide much of the code I find myself repeating for each db call in Dapper. (namely the Sql Connection, try, default catch and finally) Essentially I would like to do something like the following code but I understand that because of there being a dynamic parameter I can't use generics int this way.

The way it is I get the error:

Consider casting the dynamic arguments or calling the extension method without the extension method syntax (refering to the conn.Query method)

Is there a way to refactor my ExecuteQuery or to something like it that works?

public abtract class IDbAccessService
{
   public LogService Logger { get; set; }

   public virtual IEnumerable<T> ExecuteQuery<T>(string sql, dynamic param, string connString)
      where T : BaseModel
   {
      using (var conn = DataAccessHelpers.GetOpenConnection(connString))
      {
         try
         {
            return conn.Query<T>(sql, param).ToList<T>();
         }
         catch (Exception ex)
         {
            Logger.Logger.Error(ex.Message, ex);
            throw ex;
         }
      }
   }
}
Terrance
  • 11,764
  • 4
  • 54
  • 80
  • 1
    That `finally` is completely useless. `Close()` and `Dispose()` do the same thing (at least usually, I don't know Dapper). And that `using` will call `Dispose()` one more time. – svick Mar 04 '13 at 22:51
  • Unrelated to the question, but I think that the `finally` block is not needed - the `using` already takes care of always disposing (and hence closing) the connection – MiMo Mar 04 '13 at 22:52
  • @svick agree, and `catch` is also useless. It creates a log entry, and throws exception without stacktrace to be logged once again later – Sergey Berezovskiy Mar 04 '13 at 22:53
  • 1
    this code does not compile. You're declaring an `interface` and then adding code to it. Change that to an `abstract class` or something. Also, I think you'd be better off if you added the type parameter `T` to the class / interface itself and removed it from the method (thus having a method `public virtual IEnumerable ExecuteQuery(string sql, dynamic param, string connString)` – Federico Berasategui Mar 04 '13 at 22:53
  • Also, it would help if you posted the whole error message, not just a part of it. And also include the code that's causing that error (or say which line of the code you already posted causes it). – svick Mar 04 '13 at 22:55
  • Yes, the Close() and Dispose() calls are redundant, being implied by the using block. – Pieter Geerkens Mar 04 '13 at 22:55

2 Answers2

1

Extension methods cannot be dynamically dispatched. So call it without extension method syntax:

static IEnumerable<T> ExecuteQuery<T>(string sql, dynamic param, string connStr)
    where T : BaseModel
{
    using (var conn = DataAccessHelpers.GetOpenConnection(connStr))
    {
        return SqlMapper.Query(conn, sql, param).ToList<T>();
    }
}

Also you have useless logging which creates multiple log entries for single error, and useless connection disposing (it is done automatically by using block).

Tips for exception handling:

  • Handle exception and log it
  • Wrap exception in high-level exception and throw that wrapper (caller will handle that high-level exception, or will also wrap it)
  • Do not catch exception (caller will do first or second option)
Sergey Berezovskiy
  • 232,247
  • 41
  • 429
  • 459
1

I tried to create helper methods as below.

private SqlConnection GetSqlConnection()
        {
            var sqlConnection = new SqlConnection(ConfigurationManager.ConnectionStrings["con1"].ConnectionString);
            sqlConnection.Open();
            return sqlConnection;
        }

public IEnumerable<T> GetAll<T>(string query, object cmdParams = null, CommandType cmdType = CommandType.Text) where T : class
        {
            IEnumerable<T> objList;
            using (var conn = GetSqlConnection())
            {
                objList = conn.Query<T>(query, param: cmdParams, commandTimeout:0, commandType: cmdType);
                conn.Close();
            }
            return objList;
        }

 public IEnumerable<dynamic> Query(string query, object cmdParams = null, CommandType cmdType = CommandType.Text)
        {
            IEnumerable<dynamic> objDyn;
            using (var conn = GetSqlConnection())
            {
                objDyn = conn.Query(query, cmdParams, commandTimeout: 0, commandType: cmdType);
                conn.Close();
            }
            return objDyn;
        }

From another layer:

var param = new DynamicParameters();
param.Add("@name", name);
var objGroupsList = _iDapper.GetAll<CustomerDTO>("dbo.GetCustomersList", param, CommandType.StoredProcedure).ToList();
Sunny
  • 4,765
  • 5
  • 37
  • 72