Question

Today I noticed a snippet of code that looks like the one below:

public class Test
{
    SqlConnection connection1 = 
      new SqlConnection(ConfigurationManager.ConnectionStrings["c1"].ToString());
    SqlConnection connection2 = 
      new SqlConnection(ConfigurationManager.ConnectionStrings["c2"].ToString());

    public void Method1()
    {
        using (connection1)
        {
            connection1.Open();
            using (SqlCommand newSqlCommand = new SqlCommand("text",connection2))
            {
            // do something       
            }         
        }
    }

    public void Method2()
    {
        using (connection1)
        {
            // do something                
        }
    }
}

I am just wondering why would anyone want to open the connection when creating the class and not when calling the corresponding methods inside the class?

EDIT: I should have probably posted the whole code instead. So I do see where they are opening connection1, but then they are instantiating a sqlcommand with a different sql connection (connection2) which has not been opened anywhere. What am I missing here?

Thanks,

Was it helpful?

Solution

This line only initializes a connection object, which can later be used to open a connection to a database server.

SqlConnection connection1 = 
new SqlConnection(ConfigurationManager.ConnectionStrings["c1"].ToString());

What I know is that using disposes of an object (and in the case of Connection objects they are automatically closed) after its scope so I wouldn't recommend such usage because it might be problematic with other object types (other than Connection) that can't be used after having their dispose called.

OTHER TIPS

connection1 = new SqlConnection(...) does not really open the connection. It just creates the connection object.

You have to call connection1.Open(); to actually open it. You can do this inside using statement block.

Refer to this MSDN page for more details.

It either

  • written this way to enforce only single call to a method be performed on the class
  • unintentionally confusing by throwing ObjectDisposed exception if you call 2 methods
  • contains connection re-initialization code in blocks you've ommited.

The code is dangerous. If you call Method1 then Method2 (or visa versa) you will get an error (connection string not initialized). The error occurs because the using statement will close the connection AND Disposes the object. I double checked what happens when dispose is called...the connection string is cleared (and possibly some other things I didn't notice).

You don't want to re-use a disposed object.

The cost of instantiating a new connection object is insignificant so I would just create the object where needed, maybe with a little factory method to reduce code duplication, so change to something like:-

    private static SqlConnection GetSqlConnection()
    {
        return new SqlConnection(ConfigurationManager.ConnectionStrings["c1"].ToString());
    }

    private void Method1()
    {
        using (var conn = GetSqlConnection())
        {
            conn.Open();
            // do stuff...
        }
    }
    private void Method2()
    {
        using (var conn = GetSqlConnection())
        {
            conn.Open();
            // do other stuff...
        }
    }

Of course there are many different ways of approaching this problem, this is just one and indeed quite a simplistic one...but it is a good starting point and is safe :-)

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top