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();
}
}
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#
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:
- 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 - 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?
- 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
- Your code basically looks fine - I don't have any clue why you care about too much
using
statements :) ... except pt. 1
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
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);
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.
精彩评论