Having a disagreement with a coworker, and I at this point, I don't care who is right, I'm more curious which is the better solution so that I can use it going forward.
We have different ways to access a system.
Option #1: Creates the database with the code below.
using Microsoft.Practices.EnterpriseLibrary.Data;
namespace Ivans.Healthcare.CommercialAccess.Data
{
public abstract class DataAccess : DataHelperBase
{
public const int commandTimeout = 7200;
private static Database m_db = null;
public StringBuilder Status {get; set;}
public DataAccess()
{
this.Status = new StringBuilder();
if (m_db == null)
{
bool bIfRunsOnWebService = false;
try
{
if (DynamicConfigurationManager.AppSettings["WebService"] != null)
{
bIfRunsOnWebService = true;
}
}
catch {}
if (!bIfRunsOnWebService)
{
m_db = DatabaseFactory.CreateDatabase(DataAccessResource.IDS_DB_ALIAS);
}
else
{
m_db = CreateDatabase(DataAccessResource.IDS_WS_DB_ALIAS);
}
}
}
Then everytime a stored procedure needs to be called, the method would contain something like this:
public IEnumerable<InquiryServiceType> GetActive(bool is5010)
{
Database db = getDB();
DbCommand dbCmd = db.GetStoredProcCommand(DataAccessResource.IDS_SP_SEL_InquiryServiceTypeData_ListServiceTypes);
db.AddInParameter(dbCmd, DataAccessResource.IDS_SP_SEL_InquiryServiceTypeData_ListServiceTypes_Is5010Request, DbType.Boolean, is5010);
DataSet ds = new DataSet();
db.LoadDataSet(dbCmd, ds, new string[] { DataAccessResource.IDS_TBL_InquiryServiceTypeData });
return DataSetTranslator.TranslateInquiryServiceTypeDataSet(ds);
}
Option 2
This option is a bit more modular and was trying to create a generic database method.
private Database currentDB;
private const int commandTimeout = 7200;
public DataAccess(Common.Enums.ConnectionString currentConnection)
{
currentDB = DatabaseFactory.CreateDatabase(currentConnection.ToDescription());
}
public IEnumerable<T> SelectMany<T>(string spName, params Param[] parameters) where T : IDataPopulate<T>, new()
{
var storedProcedure = CreateStoredProcedureCommand(spName);
AddParameters(storedProcedure, parameters);
IDataReader myReader = null;
IList<T> listOfItems = new List<T>();
try
{
myReader = currentDB.ExecuteReader(storedProcedure);
if (myReader == null)
{
return listOfItems;
}
while (myReader.Read())
{
listOfItems.Add(new T().FillObject(myReader));
}
return listOfItems;
}
catch (Exception ex)
{
string message = string.Format("Error Message: {0}\r\nStored Procedure: {1}\r\n", ex.ToString(), spName);
throw new Exception(message);
}
finally
{
DataAccessDisposal.DataReader(myReader);
DataAccessDisposal.StoredProcedure(storedProcedure);
}
}
Then calling the database would look like this:
public IEnumerable<InquiryServiceTypes> GetAll(int payerID)
{
Param payerIdParam = new Param("@payerID", DbType.Int32, payerID);
return dataAccess.SelectMany<InquiryServiceTypes>("dbo.proc_PayersInquiryServiceTypesSel", payerIdParam);
}
Conclusion
There are definitely things that are coded incorrectly in each of the sections. I'm pretty sure that there is a middle ground to be reached that is the most efficient code.
The code above has two points of inefficiency. The first is how it connects to the database in the first place. The second is once the data is returned, how do you deal with it. I'd love to discuss both, but feel that the first is more important for this point.
Thanks, C
I would say simply: there are abstractions that do this already (and do it very well). If you can handle creating the connection then, for example, with dapper-dot-net:
return connection.Query<InquiryServiceTypes>(
"dbo.proc_PayersInquiryServiceTypesSel",
new { payerId }, commandType: CommandType.StoredProcedure);
which will write all the parameterisation and materialization for you in heavily cached IL. No need to write a complex Fill method or populate method, and very very fast (identical performance to writing all the ADO.NET reader code manually, but without the boring code and chance of typos).
Writing all those Fill methods manually is not efficient (for developer time).
Note in the above; the anonymous type is defining the parameters, i.e. it is saying "there is an int parameter called payerId, with the same value as was passed in". You could also have:
new { id = payerId, name = "abc", allData = true }
which would add @id (int) with the value from payerId, @name (nvarchar) with value 'abc', and @allData (bit) with value 1.
Edit re the points in the comments: