0

Are ALL the sqlCommand, sqlTransaction and sqlDataAdapter objects automatically disposed on exit from the "using" block (in which sqlConnection disposed) or all these objects to be disposed should be in separated nested "using" blocks, otherwise these objects will not be disposed when sqlConnection object disposed? What are the best practices?

public static void ExecuteDataset(string connectionString, string storedProcedure, SqlParameter[] sqlParameters = null)
{
    SqlTransaction sqlTransaction = null;

    using (SqlConnection sqlConnection = new SqlConnection(connectionString))
    {
        try
        {
            sqlConnection.Open();

            SqlCommand sqlCommand = new SqlCommand(commandText, connection);
            command.Parameters.Add("@ID", SqlDbType.Int);

            sqlCommand.Connection = sqlConnection;
            SqlTransaction = sqlConnection.BeginTransaction();
            sqlCommand.Transaction = sqlTransaction;
            .....................................................
            .....................................................    

            SqlDataAdapter sqlDataAdapter = new SqlDataAdapter();

                ................................
                ................................


            sqlTransaction.Commit();
        }
        catch (Exception)
        {
            sqlTransaction.Rollback();
        }
    }        
}
Ilan
  • 989
  • 2
  • 12
  • 22
  • 1
    General rule: if it's IDisposable, then Dispose of it when you're done – lc. Nov 15 '17 at 08:01
  • @ic. Ok, but my question is whether disposing of sqlConnection automatically disposes all the rest objects (slqTransaction, etc) or they should be in nested "using" blocks? – Ilan Nov 15 '17 at 08:11
  • 1
    Which is why I wrote as a comment instead of an answer :). In general though I don't worry about the inner workings of things, especially since they're prone to change with each version. If something I create is disposable it means I should dispose it. – lc. Nov 15 '17 at 08:16
  • @Is it mean that all these objects should be in separate nested "using" blocks, otherwise disposing of sqlConnection will not dispose them? – Ilan Nov 15 '17 at 08:21
  • The important objects are connections and transactions. Leaving a connection open keeps any accumulated database locks and transactions. Closing it releases the locks immediatelly and rolls back the transactions. *Transactions* should always be disposed for the same reason. Disposing a transaction rolls it back unless `Commit` was called. The other objects "simply" waste local memory – Panagiotis Kanavos Nov 15 '17 at 10:39
  • You can create a SqlCommand once,store it in a field and reuse it with different connections. – Panagiotis Kanavos Nov 15 '17 at 10:41
  • @PanagiotisKanavos Not neccessarily true. The database may impose limits on open cursors that may make disposing Commands and Readers mandatory, too. Just dispose anything that's IDisposable as soon as you can and you are good. – nvoigt Nov 15 '17 at 10:52
  • @nvoigt if you close the connection and transaction, the cursor is gone. Disposing the *command* isn't necessary, especially if you reuse it. You'd only need to dispose a *DbDataReader* if you kept the connection open, which should (and can) be avoided – Panagiotis Kanavos Nov 15 '17 at 11:04
  • @nvoigt does it mean that when connection disposed (on exit of the "using block"), also all other related objects (command, transaction, etc) are disposed automatically? – Ilan Nov 15 '17 at 11:06
  • @Ilan no, it means that the expensive *server side* resources (buffers, cursors, transactions, locks) are released. Their cost is 1M times greater than the memory consumed by a DbCommand object - they can cause deadlocks – Panagiotis Kanavos Nov 15 '17 at 11:07
  • @PanagiotisKanavos If you keep the commands, then after you open enough, the database will error out if it limits open cursors. Sure if you have a tiny application with enough DB resources, that will never happen, but if you have limited DB resources and an actual business app, it's better to dispose things as soon as you no longer need them. That's the whole **point** of the disposable interface. – nvoigt Nov 15 '17 at 11:09
  • @PanagiotisKanavos So in the example above I should put all the rest objects (command, transaction, etc) in separate nested "using" blocks? – Ilan Nov 15 '17 at 11:09
  • @nvoigt Did I understand correctly, connection disposing will no dispose automatically transaction, command, data adapter, etc used with this connection? – Ilan Nov 15 '17 at 11:11
  • 2
    @Ilan That is an implementation detail that you should not need to know. Dispose anything your create that is Disposable, preferably with a `using` block. Anything else is a shortcut that you *can* do... but it's simply sloppy programming and will fall apart if you change something (like for example your database provider). – nvoigt Nov 15 '17 at 11:17
  • @nvoigt Ok, but I still didn't understand in the example above, there are connection, transaction and command, when the connection disposed (on exit from "using" block), does in mean that also command and transaction will disposed automatically or they will remain open? – Ilan Nov 15 '17 at 11:23
  • 2
    Maybe. Maybe not. That's the point. You don't know. I don't know. Even if we looked up the implementation to know, the database vendor could change it with the next patch. What we do know is that each of the classes implement `IDisposable` so you should put them in a using block. That way, they will be handled properly, whatever the database vendor considers "properly". – nvoigt Nov 15 '17 at 11:26
  • @nvoigt Understood. Thank you for the explanations. – Ilan Nov 15 '17 at 11:39
  • I wouldn't use SqlTransaction here. It's meaningless in this context. It's only useful when you are sending multiple commands to the database and need an "all or nothing" - either everything succeed or everything fails. It only waist resources when used with a single sql command. – Zohar Peled Nov 17 '17 at 10:23

1 Answers1

3

If the object implements IDisposable then use using. The underlying implementation details of SqlConnection or SqlCommand or any other object should not matter.

For your scenario the best-practice would be to

using(var conn = new SqlConnection(...)) {
  ...
  using(var cmd = conn.CreateCommand(...)) {
    ...
  }
  ...
}
user2321864
  • 2,207
  • 5
  • 25
  • 35