nested using statement or try catch finally?

A

anyMouse

Out of the 3 following examples, which would be considered "best
practice"?


private SqlConnection Conn; // Connection set in class constructor


example 1
---------
internal DataTable GetDataAsDatatable(string qry)
{
using(Conn)
using(SqlCommand cmd = new SqlCommand())
using(SqlDataAdapter adpt = new SqlDataAdapter(cmd))
using(DataTable dt = new DataTable())
{
Conn.Open();
cmd.Connection = Conn;
cmd.CommandText = qry;
adpt.Fill (dt);
return dt;
}
}

example 2
---------

internal DataTable GetDataAsDatatable(string qry)
{
using(Conn){
using(SqlCommand cmd = new SqlCommand()){
using(SqlDataAdapter adpt = new SqlDataAdapter(cmd)){
using(DataTable dt = new DataTable())
{
Conn.Open();
cmd.Connection = Conn;
cmd.CommandText = qry;
adpt.Fill (dt);
return dt;
}
}
}
}
}


example 3
---------


internal DataTable GetDataAsDatatable(string qry)
{
DataTable dt = null;
SqlCommand cmd = null;
SqlDataAdapter adpt = null;
try
{
Conn.Open();
cmd = new SqlCommand();
cmd.Connection = Conn;
cmd.CommandText = qry;
adpt = new SqlDataAdapter (cmd);
dt = new DataTable();
adpt.Fill (dt);
}
catch(SqlException sqlEx)
{
throw sqlEx;
}
catch(Exception ex)
{
throw ex;
}
finally
{
if(adpt != null) {adpt.Dispose();}
if(cmd != null) {cmd.Dispose();}
if(dt != null) {dt.Dispose();}
if(Conn != null) {Conn.Dispose();}
}
return dt;
}
 
D

David Browne

Out of the 3 following examples, which would be considered "best
practice"?


private SqlConnection Conn; // Connection set in class constructor


example 1
---------
internal DataTable GetDataAsDatatable(string qry)
{
using(Conn)
using(SqlCommand cmd = new SqlCommand())
using(SqlDataAdapter adpt = new SqlDataAdapter(cmd))
using(DataTable dt = new DataTable())
{
Conn.Open();
cmd.Connection = Conn;
cmd.CommandText = qry;
adpt.Fill (dt);
return dt;
}
}
Click to expand...

As usual the best one is the lexically simplest one.

Except there's a bug: You're disposing the return value. Of course
DataTable.Dispose happens to be a NOOP, but still it's not good form.

internal DataTable GetDataAsDatatable(string qry)
{
using(Conn)
using(SqlCommand cmd = new SqlCommand())
using(SqlDataAdapter adpt = new SqlDataAdapter(cmd))
{
DataTable dt = new DataTable()
Conn.Open();
cmd.Connection = Conn;
cmd.CommandText = qry;
adpt.Fill (dt);
return dt;
}
}

David
 
M

Mattias Sjögren

Out of the 3 following examples, which would be considered "best
practice"?
Click to expand...

I would have to say none of them. #3 has a number of problems. #1 and
#2 would be OK if it wasn't for the fact that the DataTable object
you're returning will be disposed. I don't think DataTable actually
implements Dispose so in this case it might be harmless, but in
general it's an error to do so. If you remove he using statement for
the DataTable from #1 or #2, either of them will work. Which one you
choose depends on how much you like indenting your code.




Mattias
 
G

Guest

I am not sure I like the way you have done any of these completely. Let's run
through a brief primer. 'using', as you probably know is essentially the same
as:

try
{
//stuff
}
finally
{
usedObject.Dispose();
}

Don't believe me? Well, you are right. In IL, it actually is 2 additional
lines when you use Using over try ... finally. Here is some code:

private static void test1()
{
SqlConnection conn = new
SqlConnection("server=(local);database=pubs;UID=sa;PWD=sa;");
SqlCommand cmd = new SqlCommand("SELECT * FROM Authors", conn);
cmd.CommandType = CommandType.Text;

DataSet ds = new DataSet("pubs");
SqlDataAdapter da = new SqlDataAdapter(cmd);

da.TableMappings.Add("Table", "authors");

try
{
conn.Open();
da.Fill(ds);
}
finally
{
conn.Dispose();
}
}

private static void test2()
{
using(SqlConnection conn = new
SqlConnection("server=(local);database=pubs;UID=sa;PWD=sa;"))
{
SqlCommand cmd = new SqlCommand("SELECT * FROM Authors", conn);
cmd.CommandType = CommandType.Text;

DataSet ds = new DataSet("pubs");
SqlDataAdapter da = new SqlDataAdapter(cmd);

da.TableMappings.Add("Table", "authors");

conn.Open();
da.Fill(ds);
}
}

Now for the IL created (notice that test2 (using) is a bit larger):

NOTE: I used Lutz Roeder's reflector for this:

..method private hidebysig static void test1() cil managed
{
// Code Size: 94 byte(s)
.maxstack 3
.locals (
[System.Data]System.Data.SqlClient.SqlConnection connection1,
[System.Data]System.Data.SqlClient.SqlCommand command1,
[System.Data]System.Data.DataSet set1,
[System.Data]System.Data.SqlClient.SqlDataAdapter adapter1)
L_0000: ldstr "server=(local);database=pubs;UID=sa;PWD=sa;"
L_0005: newobj instance void
[System.Data]System.Data.SqlClient.SqlConnection::.ctor(string)
L_000a: stloc.0
L_000b: ldstr "SELECT * FROM Authors"
L_0010: ldloc.0
L_0011: newobj instance void
[System.Data]System.Data.SqlClient.SqlCommand::.ctor(string,
[System.Data]System.Data.SqlClient.SqlConnection)
L_0016: stloc.1
L_0017: ldloc.1
L_0018: ldc.i4.1
L_0019: callvirt instance void
[System.Data]System.Data.SqlClient.SqlCommand::set_CommandType([System.Data]System.Data.CommandType)
L_001e: ldstr "pubs"
L_0023: newobj instance void
[System.Data]System.Data.DataSet::.ctor(string)
L_0028: stloc.2
L_0029: ldloc.1
L_002a: newobj instance void
[System.Data]System.Data.SqlClient.SqlDataAdapter::.ctor([System.Data]System.Data.SqlClient.SqlCommand)
L_002f: stloc.3
L_0030: ldloc.3
L_0031: callvirt instance
[System.Data]System.Data.Common.DataTableMappingCollection
[System.Data]System.Data.Common.DataAdapter::get_TableMappings()
L_0036: ldstr "Table"
L_003b: ldstr "authors"
L_0040: callvirt instance
[System.Data]System.Data.Common.DataTableMapping
[System.Data]System.Data.Common.DataTableMappingCollection::Add(string,
string)
L_0045: pop
L_0046: ldloc.0
L_0047: callvirt instance void
[System.Data]System.Data.SqlClient.SqlConnection::Open()
L_004c: ldloc.3
L_004d: ldloc.2
L_004e: callvirt instance int32
[System.Data]System.Data.Common.DataAdapter::Fill([System.Data]System.Data.DataSet)
L_0053: pop
L_0054: leave.s L_005d
L_0056: ldloc.0
L_0057: callvirt instance void
[System]System.ComponentModel.Component::Dispose()
L_005c: endfinally
L_005d: ret
.try L_0046 to L_0056 finally handler L_0056 to L_005d
}


method private hidebysig static void test2() cil managed
{
// Code Size: 97 byte(s)
.maxstack 3
.locals (
[System.Data]System.Data.SqlClient.SqlConnection connection1,
[System.Data]System.Data.SqlClient.SqlCommand command1,
[System.Data]System.Data.DataSet set1,
[System.Data]System.Data.SqlClient.SqlDataAdapter adapter1)
L_0000: ldstr "server=(local);database=pubs;UID=sa;PWD=sa;"
L_0005: newobj instance void
[System.Data]System.Data.SqlClient.SqlConnection::.ctor(string)
L_000a: stloc.0
L_000b: ldstr "SELECT * FROM Authors"
L_0010: ldloc.0
L_0011: newobj instance void
[System.Data]System.Data.SqlClient.SqlCommand::.ctor(string,
[System.Data]System.Data.SqlClient.SqlConnection)
L_0016: stloc.1
L_0017: ldloc.1
L_0018: ldc.i4.1
L_0019: callvirt instance void
[System.Data]System.Data.SqlClient.SqlCommand::set_CommandType([System.Data]System.Data.CommandType)
L_001e: ldstr "pubs"
L_0023: newobj instance void
[System.Data]System.Data.DataSet::.ctor(string)
L_0028: stloc.2
L_0029: ldloc.1
L_002a: newobj instance void
[System.Data]System.Data.SqlClient.SqlDataAdapter::.ctor([System.Data]System.Data.SqlClient.SqlCommand)
L_002f: stloc.3
L_0030: ldloc.3
L_0031: callvirt instance
[System.Data]System.Data.Common.DataTableMappingCollection
[System.Data]System.Data.Common.DataAdapter::get_TableMappings()
L_0036: ldstr "Table"
L_003b: ldstr "authors"
L_0040: callvirt instance
[System.Data]System.Data.Common.DataTableMapping
[System.Data]System.Data.Common.DataTableMappingCollection::Add(string,
string)
L_0045: pop
L_0046: ldloc.0
L_0047: callvirt instance void
[System.Data]System.Data.SqlClient.SqlConnection::Open()
L_004c: ldloc.3
L_004d: ldloc.2
L_004e: callvirt instance int32
[System.Data]System.Data.Common.DataAdapter::Fill([System.Data]System.Data.DataSet)
L_0053: pop
L_0054: leave.s L_0060
L_0056: ldloc.0
L_0057: brfalse.s L_005f
L_0059: ldloc.0
L_005a: callvirt instance void [mscorlib]System.IDisposable::Dispose()
L_005f: endfinally
L_0060: ret
.try L_000b to L_0056 finally handler L_0056 to L_0060
}

The added IL:

L_0057: brfalse.s L_005f
L_0059: ldloc.0

Note that this is only one using, unlike your stacked using loops.

My thoughts about connections? Open as late as possible and shut as quickly
as possible. The way you have things set up, the connection is open longer
than it has to be in each of your implementations. When using a DataSet, it
should be open, pull data, dispose in quick order.

--
Gregory A. Beamer
MVP; MCP: +I, SE, SD, DBA

***************************
Think Outside the Box!
***************************
 
A

anyMouse

Thanks for your replies.

Please could you clarify why it is wrong (referring to these examples)
to dispose the DataTable which is also returned.

Lets say these examples belong to a data access class, and are called
from an asp.net client.

so...
MyDataClass MyDal = new MyDataClass()
DataTable LocalDataTable =
MyDal.GetDataAsDatatable(StoredProcedureName)
.....
do stuff with LocalDataTable

In this example, when DataTable "dt" is returned from examples 1,2 or
3, should it not be disposed as a new DataTable, "LocalDataTable" has
been created and is what I want to work with - or am I missing
something! Is LocalDataTable actually referencing "dt"?

Secondly, in this MyDataClass example, Conn is created in the
constructor:
this.Conn = new
SqlConnection(ConfigurationSettings.AppSettings.Get("LoginDetails"));


However, Gregory has pointed out that Connections should be open and
closed as quick as possible. For examples 1,2 and 3, would it be more
efficient therefore to create the connection as opposed to in the class
constructor?

Thanks for your help with this
 
M

Mattias Sjögren

Is LocalDataTable actually referencing "dt"?
Click to expand...

Yes, since DataTable is a reference type.

However, Gregory has pointed out that Connections should be open and
closed as quick as possible. For examples 1,2 and 3, would it be more
efficient therefore to create the connection as opposed to in the class
constructor?
Click to expand...

That would be a good idea, yes.



Mattias
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Sql server deadlock retry 6
Strongly typed DataTables 1
tables search 2
Command or Adapter weird error 2
Update Datatable not working 1
Update a dataTable to DB 1
return value from store procedure 3
Clr trigger for insert 9

Top