Data Access Layer to return Data Transformation object

Question

I am trying to create a Data Access Layer using ADO.Net. I have created a generic method for ExecuteNonQuery. The stored procedure I am calling will return two parameters. The output parameters from the stored procedure should be assigned to DTO object and the DTO should need to be returned to BLL.

I am a bit concerned about two things: 1) It is working only when we initialize the output parameters before calling the stored procedure 2) Converting the results to DTO object. Any performance issues?

I am not sure if it's the right approach to return the DTO object to BLL. DBHelper and DBAccess class seems ok to me. I am concerned about the GetRefNo method for the above two questions.

Could anyone please review and let me know if any changes can be made to the existing process.

public SubmissionHeaderDTO GetRefNo()
{
 var inPrms = new Dictionary<string, object>()
 {
 {"FormId", Guid.NewGuid()},
 {"FormCode", "TST"},
 {"FormTitle", "Test form"},
 {"User", "test"},
 {"Host", "iisserver"}
 };
 var outPrms = new Dictionary<string, object>() 
 {
 { "SubmissionId", Guid.NewGuid() },
 { "SubmitSequence", "one"}
 };
 var refNo = DBHelper.ExecSP(Constants.SPNames.SubmissionHeaderInsert, inPrms, outPrms);
 SubmissionHeaderDTO result = DictionaryToObject<SubmissionHeaderDTO>(refNo);
 return result;
}
private static T DictionaryToObject<T>(IDictionary<string, object> dict) where T : new()
{
 var t = new T();
 PropertyInfo[] properties = t.GetType().GetProperties();
 foreach (PropertyInfo property in properties)
 {
 if (!dict.Any(x => x.Key.Equals(property.Name, StringComparison.InvariantCultureIgnoreCase)))
 continue;
 KeyValuePair<string, object> item = dict.First(x => x.Key.Equals(property.Name, StringComparison.InvariantCultureIgnoreCase));
 Type tPropertyType = t.GetType().GetProperty(property.Name).PropertyType;
 Type newT = Nullable.GetUnderlyingType(tPropertyType) ?? tPropertyType;
 object newA = Convert.ChangeType(item.Value, newT);
 t.GetType().GetProperty(property.Name).SetValue(t, newA, null);
 }
 return t;
}

DBHelper.cs

public static Dictionary<string, object> ExecSP(string sp, Dictionary<string, object> paramIn, Dictionary<string, object> paramOut)
{
 var dbAccess = new DBAccess();
 var results = dbAccess.ExecuteQuery(sp, paramIn, paramOut);
 return results;
}

DBAccess.cs

public Dictionary<string, object> ExecuteQuery(string storedProcedure, Dictionary<string, object> parameters,
 Dictionary<string, object> outParameters)
{
 SqlConnection sqlConn = null;
 SqlCommand sqlcmd = null;
 SqlTransaction transaction = null;
 SqlParameter sqlPar = null;
 Dictionary<string, object> returnParameters = null;
 try
 {
 sqlConn = new SqlConnection(Configuration.DBConnection);
 sqlConn.Open();
 transaction = sqlConn.BeginTransaction("Results");
 sqlcmd = new SqlCommand(storedProcedure, sqlConn, transaction);
 sqlcmd.CommandType = CommandType.StoredProcedure;
 foreach (KeyValuePair<string, object> kv in parameters)
 {
 sqlcmd.Parameters.AddWithValue(kv.Key, kv.Value);
 }
 foreach (KeyValuePair<string, object> kv in outParameters)
 {
 sqlPar = sqlcmd.Parameters.AddWithValue(kv.Key, kv.Value);
 sqlPar.Direction = ParameterDirection.Output;
 }
 sqlcmd.ExecuteNonQuery();
 returnParameters = GetOutputParameters(sqlcmd.Parameters);
 transaction.Commit();
 }
 catch (System.Exception e)
 {
 var tt = e.Message;
 try
 {
 if (transaction != null)
 transaction.Rollback();
 }
 catch { }
 throw;
 }
 finally
 {
 if (sqlConn != null)
 {
 if (sqlConn.State != ConnectionState.Closed)
 {
 sqlConn.Close();
 }
 }
 if (transaction != null)
 {
 IDisposable dispose = transaction as IDisposable;
 if (dispose != null) { dispose.Dispose(); }
 }
 if (sqlcmd != null)
 {
 IDisposable dispose = sqlcmd as IDisposable;
 if (dispose != null) { dispose.Dispose(); }
 }
 if (sqlConn != null)
 {
 IDisposable dispose = sqlConn as IDisposable;
 if (dispose != null) { dispose.Dispose(); }
 }
 }
 return returnParameters;
}
private Dictionary<string, object> GetOutputParameters(SqlParameterCollection paramCollection)
{
 var returnParameters = new Dictionary<string, object>();
 foreach (SqlParameter par in paramCollection)
 {
 if ((par.Direction == ParameterDirection.Output) || (par.Direction == ParameterDirection.ReturnValue))
 {
 returnParameters.Add(par.ParameterName, par.Value);
 }
 }
 return returnParameters;
}

Answer 1

(See also comments in code)

GetRefNo

Create case-insensitive dictionaries right away as you ignore the case later anyway:

public SubmissionHeaderDTO GetRefNo()
{
 // tell the dictionaries how to compare the keys
 var inPrms = new Dictionary<string, object>(StringComparer.InvariantCultureIgnoreCase)
 {
 // ...
 };
 var outPrms = new Dictionary<string, object>(StringComparer.InvariantCultureIgnoreCase)
 {
 // ...
 };
 var refNo = DBHelper.ExecSP(Constants.SPNames.SubmissionHeaderInsert, inPrms, outPrms);
 SubmissionHeaderDTO result = DictionaryToObject<SubmissionHeaderDTO>(refNo);
 return result;
}

DictionaryToObject

You don't need to call .GetType().GetProperties() over and over. You already have retrieved the properties at the beginning. It might be a real performance hit if you do it frequently. Luckily this can be easily avoided.

private static T DictionaryToObject<T>(IDictionary<string, object> dict) where T : new()
{
 // use a meaningfull name for the instance of T
 var result = new T();
 var properties = 
 typeof(T)
 .GetProperties()
 // filter the properties hier not in the loop, it's easier to modify and understand
 .Where(p => dict.ContainsKey(p.Name));
 foreach (var property in properties)
 { 
 // you can now directly get the value for the property
 var value = dict[property.Name]
 var propretyType = Nullable.GetUnderlyingType(property.PropertyType) ?? property.PropertyType;
 var convertedValue = Convert.ChangeType(value, propretyType);
 // there's no need to GetType.GetProperties agian, just set the value
 property.SetValue(result, convertedValue)
 }
 return result;
}

ExecuteQuery

Your code will look nicer and simpler if you use the using instead of the lengthy finally block:

public Dictionary<string, object> ExecuteQuery(
 string storedProcedure, 
 Dictionary<string, object> parameters,
 Dictionary<string, object> outParameters)
{ 
 // the usings will gracefully dispose/close everything disposable
 using (var sqlConn = new SqlConnection(Configuration.DBConnection))
 {
 sqlConn.Open();
 // if this is a select I doubt you need a transaction for this
 using (var transaction = sqlConn.BeginTransaction("Results"))
 using (var sqlcmd = new SqlCommand(storedProcedure, sqlConn, transaction))
 {
 sqlcmd.CommandType = CommandType.StoredProcedure;
 // use var where possible
 foreach (var kv in parameters)
 {
 sqlcmd.Parameters.AddWithValue(kv.Key, kv.Value);
 }
 foreach (var kv in outParameters)
 {
 // no helper variables needed here
 sqlcmd
 .Parameters
 .AddWithValue(kv.Key, kv.Value)
 .Direction = ParameterDirection.Output;
 }
 try
 {
 sqlcmd.ExecuteNonQuery();
 var result = GetOutputParameters(sqlcmd.Parameters);
 transaction.Commit();
 return result;
 }
 catch (Exception)
 {
 transaction.Rollback();
 throw;
 }
 }
 }
}

• Jesse C. Slicer: Spot-on with what I was starting to write up. One thing I was going to mention - there's a haphazard mixing of Dictionary and IDictionary in the OP's code. I would standardize on developing to interfaces and make sure things are declared as IDictionary for parameters and return types.
• t3chb0t: @JesseC.Slicer indeed, there's a small chaos. Now, two years later, I'd do it exactly as you suggest too ;-]