Should you reuse the SqlConnection, SqlDataAdapter and SqlCommand objects?

Question

I am working with a DAL object that is written in a layout similar to the following code. I just simplified the code to show the installation.

public class UserDatabase : IDisposable { private SqlDataAdapter UserDbAdapter; private SqlCommand UserSelectCommand; private SqlCommand UserInsertCommand; private SqlCommand UserUpdateCommand; private SqlCommand UserDeleteCommand; private System.Data.SqlClient.SqlConnection SQLConnection; public UserDatabase() { this.SQLConnection = new System.Data.SqlClient.SqlConnection(ConnectionString); this.UserDbAdapter= new SqlDataAdapter(); this.UserDbAdapter.DeleteCommand = this.UserDeleteCommand; this.UserDbAdapter.InsertCommand = this.UserInsertCommand; this.UserDbAdapter.SelectCommand = this.UserSelectCommand; this.UserDbAdapter.UpdateCommand = this.UserUpdateCommand; } private bool FillUsers(DataSet UserDataSet, out int numberOfRecords) { bool success = true; numberOfRecords = 0; string errorMsg = null; this.UserDbAdapter.SelectCommand = this.GetUsersSelectCommand(); numberOfRecords = UserDbAdapter.Fill(UserDataSet, UsersTableName); return success; } private SqlCommand GetUserSelectCommand() { if (this.UserSelectCommand==null) this.UserSelectCommand= new System.Data.SqlClient.SqlCommand(); this.UserSelectCommand.CommandText = "dbo.Users_Select"; this.UserSelectCommand.CommandType = System.Data.CommandType.StoredProcedure; this.UserSelectCommand.Connection = this.SQLConnection; this.UserSelectCommand.Parameters.Clear(); this.UserSelectCommand.Parameters.AddRange(new System.Data.SqlClient.SqlParameter[] { new System.Data.SqlClient.SqlParameter("@RETURN_VALUE", System.Data.SqlDbType.Variant, 0, System.Data.ParameterDirection.ReturnValue, false, ((byte)(0)), ((byte)(0)), "", System.Data.DataRowVersion.Current, null)}); return UserSelectCommand; } 

There are several other functions such as padding that are written in the same way as reusing the Connection, SqlCommands, and SqlDataAdapter objects. The SqlDataAdapter manages the opening and closing of the SqlConnection internally.

So my question is multiple. Is this design bad? If so, why?

If this is bad, should it be changed to store things in a more local area, for example:

  public bool FillUsers(DataSet UserDataSet) { using (SqlConnection conn = new SqlConnection(ConnectionString)) { using (SqlCommand command = GetUsersSelectCommand()) { using (SqlDataAdapter adapter = new SqlDataAdapter(command, conn)) { adapter.Fill(UserDataSet, UsersTableName); } } } } 

This should be done for all functions that seem to create, delete, and then remake, worse than keeping things around. However, this seems to be the setting that I see everywhere on the Internet.

Answer 1

No, there is nothing wrong with that. You must manage your objects that implement IDisposable as soon as you are done with them.

Given SqlConnection , when you delete a connection, the underlying connection will simply be returned to the pool. This is not necessarily “closed,” as you think. It is best to let the connection pool do the job. Here is a link to the MSDN connection with ADO.NET. Trying to get him to do something he’s not intended to do (some call it optimization, surprisingly), it’s usually a trip down the rabbit hole.

Also, make sure you really measure and detect the problem before trying to optimize it. (and I don't mean it in a harsh way, just save your time ).