I've recently inherited a .Net Web App with a 4 layer setup. As per normal with these situations, I have no documentation and no contact with the prior devs. The members of previous team for the app had vastly different habits, and I'm trying to sort through these as I refactor/standardize the code. I ran into something I haven't seen before (and I'm not able to find anywhere or anyone implementing this as a practice ) in which one/many of the developers wrap all DataSets in a using statement as it's returned through the layers. As soon as I saw it I wanted to junk it. In many of these implementations the readability is shot. Then I started questioning myself because I can't imagine someone doing this just because. I understand the desire to dispose of objects as they're used, but this just seems like someone went overboard. Does anyone know of a solid reason to keep this implemented?
Here's a little TLDR example of what's being done.
Presentation:
using (DataSet ds = objCustomerFBO.FetchCustomerInfo(base.KeyID))
{
grdCustomerInfo.DataSource = ds;
}
grdCustomerInfo.DataBind();
Object:
public DataSet FetchCustomerInfo(int keyID)
{
using (DataSet ds = objCustomerEBO.FetchCustomerInfo(keyID);
{
return ds;
}
}
Business:
public DataSet FetchCustomerInfo(int keyID)
{
SqlParameter arrSqlParam = new SqlParameter[1];
arrSqlParam[0] = new SqlParameter("@CustomerID", SqlDbType.Int);
arrSqlParam[0].Value = keyID;
using (DataSet ds = CommonEBO.ExecuteDataSet("FetchCustomerInfo", arrSqlParam);
{
return ds;
}
}
Data (after filtering through overloads):
public static DataSet ExecuteDataSet(string strCommandText, SqlParameter[] SqlParams, CommandType commandtype, string strConnectionString)
{
DataSet ds = new DataSet();
try
{
using (SqlDataAdapter oAdpt = new SqlDataAdapter(CMSOnlineDH.CreateCommand(strCommandText, SqlParams, commandtype, strConnectionString)))
{
oAdpt.SelectCommand.Connection.Open();
oAdpt.Fill(ds);
}
}
catch { throw; }
return ds;
}
I don't see the point in the using statements that wrap around a return of the instantiated object. These I would remove as it just hinders readability.
The first example doesn't make much sense either - since calling DataBind will need to operate over the bound DataSet, doing the binding outside of the using statement makes very little sense. I would have put that within the using statement.
The only usage that appears correct to me is with the DataAdapter (last example).
In all it looks like someone took the advice to always use using statements for objects implementing IDisposable a bit overboard.
It isn't wrong as you've inherited it and its working.
I believe ExecuteDataSet could of been the only place needed for that using statement.
Based on the information given of course. If however, you've stripped out snippets of connection information and other resources that might be a different story.
For instance, what was the purpose of the presentation layer being wrapped around a using statement that gets its value from a returned dataset???
What exactly does this impact? I think ultimately it affects other programmers (As you have posted here) in regards to readability and understanding what exactly is going in. It's additional noise as your eyes sift through all that code.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With