4

I have following statement using using:

using (var reader = data.CreateCommand(sql).ExecuteDataReader())

in this case data is some object which internally holds SqlConnection. CreateCommand(sql) function returns SqlCommand and ExecuteDataReader returns SqlDataReader. Since SqlCommand and SqlDataReader are both IDisposable, will they both be disposed by this use of using statement?

For now I have done it like this:

using (var cmd = data.CreateCommand(sql))
using (var reader = cmd.ExecuteDataReader())

But I would like to know if it's possible to combine them as stated above?

klashar
  • 2,519
  • 2
  • 28
  • 38
Jure
  • 1,156
  • 5
  • 15
  • 3
    From the information you've given us, the answer has to be that you need to do it with the separate `using`s. – Matthew Watson Feb 23 '17 at 09:32
  • Should I provide any more information? Although, I think I've also realized that it won't go with a single using. It would be nice though, especially if there were more usings ;) – Jure Feb 23 '17 at 09:43
  • is `data` a custom class you wrote to access database? i mean can you modify its code? – S.Serpooshan Feb 23 '17 at 09:58
  • @Jure - I'll write you up an answer in a second (after I double check the `SqlCommand` class' `Dispose` method and `ExecuteReader` method. However a helpful read: http://stackoverflow.com/a/1461836/3645638 **Quote:** *"Any class that instantiates an IDisposable member should implement IDisposable itself, and properly Dispose() of its constituents."* – Svek Feb 23 '17 at 10:05
  • Based on my understanding, Disposing the `SqlConnection` would dispose all resources using it, therefore `SqlCommand`, `SqlDataReader` would go with it... That being said, I believe that disposing `SqlCommand` would then dispose `SqlDataReader`. So you don't have to put the `SqlReader` in the `using` statement. – Svek Feb 23 '17 at 10:20
  • @Svek disposing `SqlConnection` will not dispose associated `SqlCommand` or reader! – S.Serpooshan Feb 23 '17 at 11:02
  • @S.Serp My answer below illustrates clearer as to what I was trying to say in the comments box. – Svek Feb 23 '17 at 11:06
  • @S. Serp: `data` is my custom object (internally it has SqlConnection for db-based stuff). CreateCommand and ExecuteDataReader are also my methods. – Jure Feb 23 '17 at 11:22

4 Answers4

4

"if it's possible to combine them" - two using are totally fine, because both needs to be disposed.

You can combine them by extracting into method:

void RunWithReader(string sql, Action<SQLDataReader> action)
{
    using (var cmd = data.CreateCommand(sql))
    using (var reader = cmd.ExecuteDataReader())
        action(reader);
}

Then you can use lambda

RunWithReader("whatever command", reader =>
{
    ... // while(reader.Read() { ... } - this could also be extracted
});
Sinatr
  • 20,892
  • 15
  • 90
  • 319
3

Agree with Matthew Watson's comment. You need to have both of usings statemets. Here is the related question with additional reasoning. SqlConnection SqlCommand SqlDataReader IDisposable

Community
  • 1
  • 1
klashar
  • 2,519
  • 2
  • 28
  • 38
  • Interesting, will the garbage collector not just remove the SqlCommand object after reader is destroyed? Edit: no, probably not, as `data` is not destroyed... – JHBonarius Feb 23 '17 at 10:01
2

The way you have presented your code the inner IDisposable (IDbCommand) is not disposed.

You have two choices:

You can put it all in one using like this:

using (IDisposable cmd = data.CreateCommand(), reader = ((IDbCommand)cmd).ExecuteReader())
{
    // code here
}

But that is rather cumbersome. The other option is nested using statements:

using (var cmd = data.CreateCommand())
{
    using (var reader = cmd.ExecuteReader())
    {
        // code here
    }
}

Other than that you can get a bit complicated and write an extension method to help (sort of) clean it up for you.

public static class Ex
{
    public static void Using<T, R>(this T obj, Func<T, R> create, Action<T, R> use) where R : IDisposable
    {
        using (var d = create(obj))
        {
            use(obj, d);
        }
    }
}

Then you can do this:

data.Using(d => d.CreateCommand(), (d, c) => c.Using(c2 => c2.ExecuteReader(), (c3, r) =>
{
    // code here
}));

But perhaps that's not much of an improvement.

Enigmativity
  • 113,464
  • 11
  • 89
  • 172
-1

In General

You should Dispose every class that implements IDisposable.

However, your question is specifically dealing with a scenario where all the resources are all related, which if my understanding is correct, the Garbage Collector will dispose these resources for you automatically.


The Finalize() Method ("Destructors" or "Finalizers")

The Finalize() method makes sure that resources are disposed when the instance is no longer referenced by the application.

In concert together with the Dispose() method

The Dispose() method will "release all resources used by the component". Which would mean that this code block (taken from MSDN's ExecuteReader Method Example) will properly dispose of all the compenents used by it...

using (SqlConnection connection = new SqlConnection(connectionString))
{
    connection.Open();

    SqlCommand command = new SqlCommand(queryString, connection);
    SqlDataReader reader = command.ExecuteReader();
    while (reader.Read())
    {
        // ...
    }
}

In this case, when disposing SqlConnection, it will properly release all resources used by the component. Which means:

  • SqlCommand
  • SqlDataReader

    ** Note: This has always been my understanding...


Refactoring of the above code to the following:

using (SqlConnection connection = new SqlConnection(connectionString))
{
    connection.Open();

    using (SqlCommand command = new SqlCommand(queryString, connection))
    {
        using (SqlDataReader reader = command.ExecuteReader())
        {
            while (reader.Read())
            {
                // ...
            }
        }
    }
}

The main advantage of manually disposing of each resource, would be for a performance gain since we are not processing up to the Finalize method (which is relatively a more expensive than doing Dispose().

However, that doesn't mean you want to Dispose() everything, as in some cases the Finalize() method is a better choice.


Assuming that I am still in good shape so far, and my understanding of the Garbage Collector's behavior is valid.

I would consider that SqlCommand (full code reference here), would then also dispose of SqlReader, which would mean that you could just get away with:

using (SqlCommand command = new SqlCommand(queryString, connection))
{
    SqlDataReader reader = command.ExecuteReader();
    while (reader.Read())
    {
        // ...
    }
}

which in your case would translate to:

using (var cmd = data.CreateCommand(sql))
{
    var reader = cmd.ExecuteDataReader();
    // ...
}
Svek
  • 12,350
  • 6
  • 38
  • 69
  • see this http://stackoverflow.com/a/1808056/2803565 . also its comment: not disposing a SqlCeCommand, for example, will cause your mobile device to run out of memory quite fast. – S.Serpooshan Feb 23 '17 at 10:53
  • @S.Serp `System.Data.SqlServerCe.SqlCeCommand` and `System.Data.SqlClient.SqlCommand` are different. – Svek Feb 23 '17 at 11:03
  • yes i know, but as said in comment, this is an example to say its not good idea to don't dispose objects which are `IDisposable`. the issues depends on several params and it is not safe! if you check the source code of `SqlCommand.Dispose`, you see it release something as: `this._cachedMetaData = null;` – S.Serpooshan Feb 23 '17 at 11:05
  • If my understanding is correct, `reader` in the last example would not get disposed, when `cmd` is disposed, because it's not in its own `using` statement? – Jure Feb 23 '17 at 11:27
  • @Jure From what I understand, it does get disposed of, as the `cmd` resouce is disposed, the `Finalize()` method would clean it up. It's not *bad* design, just not as tidy as calling the `Dispose()` method. – Svek Feb 24 '17 at 05:17