About using "use" and "final" to clean resources

Question

Is there a case when the following structure is needed?

using (Something something = new Something()) { try { } finally { something.SomeCleanup(); } } 

Or, should all cleanup tasks be performed in an implicit call to something.Dispose() ?

---

Here is the violation code:

 public static DataTable GetDataTable(string cmdText, IEnumerable<Parameter> parameters) { // Create an empty memory table. DataTable dataTable = new DataTable(); // Open a connection to the database. using (SqlConnection connection = new SqlConnection(ConfigurationTool.ConnectionString)) { connection.Open(); // Specify the stored procedure call and its parameters. using (SqlCommand command = new SqlCommand(cmdText, connection)) { command.CommandType = CommandType.StoredProcedure; SqlParameterCollection parameterCollection = command.Parameters; foreach (Parameter parameter in parameters) parameterCollection.Add(parameter.SqlParameter); try { // Execute the stored procedure and retrieve the results in the table. using (SqlDataAdapter dataAdapter = new SqlDataAdapter(command)) try { dataAdapter.Fill(dataTable); } catch { dataTable.Dispose(); dataTable = null; } } finally { //parameterCollection.Clear(); } } } return dataTable; } 

NOTE. I have defined the Parameter class, so users of this function need not directly access the creation of SqlParameter . The SqlParameter property of the Parameter class can be used to retrieve the SqlParameter .

At some point, my program does the following (I can’t publish the code, because it is connected with many classes, basically I have a mini-framework that creates many objects):

• Create an array of Parameter s.
• GetDataTable('sp_one', parameters) .
• GetDataTable('sp_two', parameters) .

Answer 1

The using keyword only calls the .Dispose() method. If you have the necessary cleanup that happens outside of the dispose method on the IDisposable, then you will need to do this in your own finally block. This leads to two points:

• At this point, you can argue that you can also skip the use block and simply call Dispose () inside the finally block. Personally, I will still go with the using block. It is just a good habit to always have one for your IDisposable instances.
• I humbly suggest that if you consider the conditions above, you need to redesign your class to use the IDisposable template.

---

Based on the code you posted, the problem is that your options were still rooted somewhere (maybe you are reusing them?). Since the parameters are still rooted, they cannot be collected. They also contain a link to the command to which they are attached, so your SqlCommand object also cannot be built right away, because now it is still rooted.

The key point is that the .Net framework reserves the Dispose () pattern for unaudited resources. Since SqlParameters and SqlParameterCollection are type-driven, they are not affected until they are assembled, which is completely separate from the deletion. When your SqlCommand is finally built, it will also take care of the SqlParameter collection. Just do not confuse the collection, removal and their goals.

What you want to do is make a copy of each parameter when you add it, and not add an existing parameter to the collection.

 public static DataTable GetDataTable(string cmdText, IEnumerable<Parameter> parameters) { // Create an empty memory table. DataTable dataTable = new DataTable(); // Prepare a connection to the database and command to execute. using (SqlConnection connection = new SqlConnection(ConfigurationTool.ConnectionString)) using (SqlCommand command = new SqlCommand(cmdText, connection)) { command.CommandType = CommandType.StoredProcedure; SqlParameterCollection parameterCollection = command.Parameters; foreach (Parameter parameter in parameters) parameterCollection.Add(CloneParameter(parameter.SqlParameter)); // Execute the stored procedure and retrieve the results in the table. using (SqlDataAdapter dataAdapter = new SqlDataAdapter(command)) { dataAdapter.Fill(dataTable); } } return dataTable; } 

Some things to note here: I was able to get rid of all your try blocks. None of them are needed. In addition, the SqlDataAdapter.Fill () method will open and close the connection for you, so you do not need this part.

Now let's talk about this CloneParameter () function. I get the impression that you feel that this violates the purpose of your code, and this is an attempt to reuse the parameters. I promise you that reusing this option is a bad idea. The performance degradation is negligible, especially compared to performing stored procedures. I left you the implementation of CloneParameter () for two reasons: first of all, this is trivial, and secondly, we are already outside of my usual data access pattern. Typically, to add parameters, I accept an Action <SqlParameterCollection> delegate, not a parameter that can be enumerated. The function is declared as follows:

 public IEnumerable<IDataRecord>GetData(string cmdText, Action<SqlParameterCollection> addParameters) 

and is called like this:

 foreach(var record in GetData("myprocedurename", p => { p.Add( /*new parameter here*/ ); p.Add( /*new parameter here*/ ); //... }) .Select( /*Returning a IEnumerable rather than a datatable allows me to use it with linq to objects.*/ /* For example, you could use this spot to convert from DataRecords returned by ADO.Net to business objects */ )) { // use the results here... } 

Since you are filling two tables in a row, it looks like you are working on the client side, which may justify this against the DataReader / IEnumerable approach, but I want to mention this, since most of the time your code is based in DataReader is the best option.

What would I do in your case with my existing Action-delegate-based template and wanting to reuse a duplicate set of parameters as much as possible, there is a real named method that knows how to add parameters and matches my delegate for actions. Then I could just pass this method and get the desired parameter reuse.

Answer 2

Interest Ask!

It all depends on your Something class. If it is poorly designed and needs to be cleaned in stages, it causes idiosyncrasy to its customers.

You should not create classes this way. If you have intermediate cleanups, encapsulate them in your own classes and use the following code:

 using (Something something = new Something()) { // ... using (SomethingElse somethingElse = something.GiveMeSomethingElse()) { } // ... } 

UPDATE:

In your example, it might look like this:

 using (SqlConnection connection = new SqlConnection(connectionString)) { connection.Open(); using (SqlCommand command = new SqlCommand("select * from MyTable where id = @id", connection)) { // to "reuse" the parameters collection population, just extract this to a separate method command.Parameters.Add(new SqlParameter("id", id)); // ... execute the command } } 

UPDATE 2:

Just do the following:

 GetDataTable('sp_one', CreateParameters()); GetDataTable('sp_two', CreateParameters()); 

Answer 3

Dispose should clear all unmanaged resources. For example, another cleaning method is possible, for example, to perform some actions with a functional or a database.