CODEWITHSUNDEEP
c#coding-styleidisposableusing
Shawn
Shawn

Reputation: 1891

C# IDisposable Using: Best Practice

I have been wrapping my OracleConnection and OracleCommand objects in USING statements for a while now, however, after running code analyzer I discover that OracleParameter also implements IDisposable. Is the following code correct? Is there a better technique for readability or structure? At first glance it just seems to be cluttered with USING statements:

using (OracleConnection conn = new OracleConnection(connectionstring))
{
    conn.Open();
    using (OracleCommand cmd = new OracleCommand(sql, conn))
    {
        cmd.BindByName = true;

        using (OracleParameter param1 = new OracleParameter("p1", OracleDbType.Int32, System.Data.ParameterDirection.Input))
        {
            param1.Value = int.Parse(value1);
            cmd.Parameters.Add(param1);
        }

        using (OracleParameter param2 = new OracleParameter("p2", OracleDbType.Varchar2, System.Data.ParameterDirection.Input))
        {
            param2.Value = value2;
            cmd.Parameters.Add(param2);
        }

        using (OracleDataReader dr = cmd.ExecuteReader())
        {
            // loop data here...
        }
    }
}

Upvotes: 8

Views: 3222

Answers (7)

MusiGenesis
MusiGenesis

Reputation: 75386

According to MSDN, you only need to use using for Connection and DataReader objects. I have never seen using (or .Dispose()) used with ADO.NET parameter objects. If this were necessary or even desirable, I think it would have come up already some time within the last 10 years.

Upvotes: 1

mfeingold
mfeingold

Reputation: 7134

This code is incorrect. the params you create they are still used outside the using statement scope because you add them to the parameter collection, but the using statement will call Dispose on the parameters upon control leaving the scope. Which means that when the time will come to use the parameters inside the command theya will have been disoised of already

Upvotes: 0

Platinum Azure
Platinum Azure

Reputation: 46233

You want to dispose of the parameters only at the very end of their use, including during the query (and possibly the reading of the results):

using (OracleConnection conn = new OracleConnection(connectionstring))
{
    conn.Open();
    using (OracleCommand cmd = new OracleCommand(sql, conn))
    {
        cmd.BindByName = true;

        using (OracleParameter param1 = new OracleParameter("p1", OracleDbType.Int32, System.Data.ParameterDirection.Input))
        using (OracleParameter param2 = new OracleParameter("p2", OracleDbType.Varchar2, System.Data.ParameterDirection.Input))
        {
            param1.Value = int.Parse(value1);
            cmd.Parameters.Add(param1);
            param2.Value = value2;
            cmd.Parameters.Add(param2);

            using (OracleDataReader dr = cmd.ExecuteReader())
            {
                // loop data here...
            }
        }
    }
}

Notice that you can put multiple using statements in a row. This is because, like the if statement,

  1. A using statement is considered a simple statement (even with a block); and
  2. A using statement can take either a block or a statement underneath.

Upvotes: 9

DarthVader
DarthVader

Reputation: 55112

can you look at the connection and command source code, does it dispose the parameters? if the connection or command objects dispose pattern wraps the parameters and dispose them upon they are disposed. you should worry about it. which i think it would/should.

Upvotes: 1

Erik Funkenbusch
Erik Funkenbusch

Reputation: 93464

using (OracleConnection conn = new OracleConnection(connectionstring)) 
using (OracleCommand cmd = new OracleCommand(sql, conn)) 
using (OracleParameter param1 = new OracleParameter("p1", OracleDbType.Int32,
       System.Data.ParameterDirection.Input)) 
using (OracleParameter param2 = new OracleParameter("p2", OracleDbType.Varchar2,
       System.Data.ParameterDirection.Input)) 
}
    conn.Open(); 
    cmd.BindByName = true; 

    param1.Value = int.Parse(value1); 
    cmd.Parameters.Add(param1); 

    param2.Value = value2; 
    cmd.Parameters.Add(param2); 

    using (OracleDataReader dr = cmd.ExecuteReader()) 
    { 
        // loop data here... 
    } 
}

Upvotes: 2

Haris Hasan
Haris Hasan

Reputation: 30127

No this is not correct because you are disposing the parameters even before you use them.

Instead you should so it like this

OracleParameter param1 = new OracleParameter("p1", OracleDbType.Int32, System.Data.ParameterDirection.Input);

param1.Value = int.Parse(value1);
cmd.Parameters.Add(param1);


OracleParameter param2 = new OracleParameter("p2", OracleDbType.Varchar2, System.Data.ParameterDirection.Input);

param2.Value = value2;
cmd.Parameters.Add(param2);


using (OracleDataReader dr = cmd.ExecuteReader())
{
  // loop data here...
}

param1.dispose();
param2.dispose();

Upvotes: 0

villecoder
villecoder

Reputation: 13494

I'm not sure that will work properly. Consider that at the end of the using, both parameters should have been disposed. The fact that your cmd.Parameters object is still holding a reference to them does not preclude what may be happening in the OracleParameter Dispose method. For all intensive purposes, the developer of that particular object may be clearing fields that your OracleCommand is expecting to be filled.

There's some danger there. If you're absolutely certain you want to dispose your OracleParameters properly, I suggest you dispose of them after the OracleDataReader using.

Remember that generally you call Dispose when you're done using the object. You're telling it to release all resources it's holding back to the pool. If you're not done using an object, don't dispose of it prematurely.

Upvotes: 1

Related Questions