"Using Statement" across N-Layer

Question

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;
    }

Someone was rather very very careful with objects...it definately isn't wrong though..I would say just keeping it on the safe side. — JonH, Jan 31 '12 at 19:24

score 2 · Accepted Answer · answered Jan 31 '12 at 19:25

2

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.

answered Jan 31 '12 at 19:25

Oded

489,969
99
883
1,009

I agree it's overboard but it isn't wrong, its just not needed. – JonH Jan 31 '12 at 19:28
I didn't say it was wrong as such. I find that in some of these examples the `using` statement hinders readability. Having a `try{}finally{}` block for the returned `DataSet`s and before calling `DataBind` doesn't look right to me either. – Oded Jan 31 '12 at 19:29
Oded I wasn't referring to you - just a general statement. I agree with what you have so +1. – JonH Jan 31 '12 at 19:30
"I don't see the point in the using statements that wrap around a return of the instantiated object. " This was my first thought, which made me want to remove the using and simply return the object. Thanks for the affirmation. – Bear In Hat Jan 31 '12 at 19:35

score 2 · Answer 2 · answered Jan 31 '12 at 19:26

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.

That was the entirety of that method. For round one of the refactoring I'll stick with not fixing what isn't broken. As for the presentation, this is only one of many unanswered questions that will be changed now. — Bear In Hat, Jan 31 '12 at 19:35

score 1 · Answer 3 · edited May 23 '17 at 12:27

1

Looks like disposing a Dataset doesn't give any benefit because IDisposable it's just not implemented (have a look here).

edited May 23 '17 at 12:27

Community

1
1

answered Jan 31 '12 at 19:32

Giorgio Minardi

2,765
1
15
11

"Using Statement" across N-Layer

3 Answers3