using "using" too much?

Question

Given the following code, i see no more need for the finally block for closing the reader or the connection (if it were still available). Are there any benefits or disadvantages to using so many nested "using" statements? Or shall i go the finally block route?

List<string> platforms = new List<string>();

NpgsqlDataReader reader = null;

try
{
    using (NpgsqlConnection conn = new NpgsqlConnection(GetConnectionString()))
    {
        // Making connection with Npgsql provider
        string sql = @"SELECT platforms.""name"" FROM public.""platforms""";

        using (NpgsqlCommand command = new NpgsqlCommand(sql))
        {
            command.Connection = conn;
            command.CommandType = System.Data.CommandType.Text;

            conn.Open();

            using (reader = command.ExecuteReader())
            {
                while (reader.Read())
                {
                    platforms.Add((string)reader["name"].ToString());

                }
            }
        }
    }
}
catch (Exception err)
{
    HandleError(err, "GetPlatforms");

}
finally
{
    platforms = null;

    if (!reader.IsClosed)
    {
        reader.Close();
    }
}

Answer 1

It ensures the release of resources when the using block is finished. Per MSDN:

The using statement allows the programmer to specify when objects that use resources should release them. The object provided to the using statement must implement the IDisposable interface. This interface provides the Dispose method, which should release the object's resources.

A using statement can be exited either when the end of the using statement is reached or if an exception is thrown and control leaves the statement block before the end of the statement.

I do not see anything wrong with the multiple using statement blocks you have listed in your code. It ensures the resources are released and that way the programmer does not forget.

If you do not like the identation, then you can re-write it something like this:

using (StreamWriter w1 = File.CreateText("W1"))
using (StreamWriter w2 = File.CreateText("W2"))
{
      // code here
}

See also this SO question on nested using statements in C#

Answer 2

do you actually know how a using get's compiled?

taking

using (var disposableObject = new DisposableObject())
{
    // do something with it
}

get's compiled to (more or less):

IDisposable disposableObject = new DisposableObject();
try
{
    // do something with it
}
finally
{
    if (disposableObject != null)
    {
        disposableObject.Dispose();
    }
}

just an idea: in which cases could an exception occur?

• db is gone: how should you handle that?
• query is wrong: ... that should be logged

a guess: i suppose NpgsqlConnection calls .Close() on .Dispose() itself - but you would have to verify that with eg. .NET Reflector

as you've asked for it via a comment:

1. I don't believe that it's a good choice to have one big catch... You explicitly know what can go wrong - split that into several catches
2. Handling those exceptions is not done with logging. Depending on the environment (stateful vs. non-stateful) you need to handle it (recreating the connection, querying again, ...). Being in a non-stateful environment retrying mostly makes no sense... When the db is gone, which alternatives do you have?
3. Is this just a cosmetic question or do you really want to know what's going on under the hood? If it's a cosmetic one ... pfuh ... If it's more to the core, I would not care about peoples answers and hunt the performance with a profiler :) AND would do some investigation with a reflection-tool
4. Your code basically looks fine - I don't have any clue why you care about too much using statements :) ... except pt. 1

Answer 3

if you are using the "using" block there is no need to use finally.

to just closing read and connection.

understanding ‘using’ block in C#

Code

using System; using System.Collections.Generic; using System.Linq; using System.Text;

namespace BlogSamples
{
    class Program
    {
        static void Main(string[] args)
        {
            using (Car myCar = new Car(1))
            {
                myCar.Run();
            }
        }
    }
}

IL of code

.method private hidebysig static void  Main(string[] args) cil managed
{
  .entrypoint
  // Code size       37 (0x25)
  .maxstack  2
  .locals init ([0] class BlogSamples.Car myCar,
           [1] bool CS$4$0000)
  IL_0000:  nop
  IL_0001:  ldc.i4.1
  IL_0002:  newobj     instance void BlogSamples.Car::.ctor(int32)
  IL_0007:  stloc.0
  .try
  {
    IL_0008:  nop
    IL_0009:  ldloc.0
    IL_000a:  callvirt   instance void BlogSamples.Car::Run()
    IL_000f:  nop
    IL_0010:  nop
    IL_0011:  leave.s    IL_0023
  }  // end .try
  finally
  {
    IL_0013:  ldloc.0
    IL_0014:  ldnull
    IL_0015:  ceq
    IL_0017:  stloc.1
    IL_0018:  ldloc.1
    IL_0019:  brtrue.s   IL_0022
    IL_001b:  ldloc.0
    IL_001c:  callvirt   instance void [mscorlib]System.IDisposable::Dispose()
    IL_0021:  nop
    IL_0022:  endfinally
  }  // end handler
  IL_0023:  nop
  IL_0024:  ret
} // end of method Program::Main

As you can see here you using block is get converted in try...finally. but the class must have to implement IDispose interface

Answer 4

I dont know about any disadvantages apart from the indentation of code. The Advantage is obviously that you do not need to worry about disposing of your objects as they will be disposed of as soon as the using brace is left.

I noticed in your error handler you are passing what looks like the name of the method. For a more generic one you could use by adding to the tool box, or creating a snippet for would be similar to the following to automatically pick up the method and class name. You can use reflection to get these details.

ErrorHandler.Handler.HandleError(ex, System.Reflection.MethodBase.GetCurrentMethod().Name, System.Reflection.MethodBase.GetCurrentMethod().DeclaringType.FullName);

Answer 5

I think your code is quite OK, but I personally would rather refactor one using clause to separate function, since having 5 consecutive curly braces (i.e. } ) makes code less readible. But this is my point of view - I like keeping level of indentation as low as possible.