I know that connection state matters, I just sometimes forget. Of course, I could also say that proper error handling matters as well, since that's the crux of this particular issue so I'll cover that as well.
Here's the crux of the matter. I am writing a class to process a queue of information and I can have several programs processing the data. Because I may have to process many different items in the queue, I've set up global SqlCommand objects and simply populate the parameters when I need to call them. Many of the methods within the class look something like this:
private void DoSomething(string VariableID)
{
cmdExecuteThis.Parameters["@VariableID"].Value = VariableID;
cmdExecuteThis.Connection.Open();
cmdExecuteThis.ExecuteNonQuery();
cmdExecuteThis.Connection.Close();
}
There are many problems with this code, but I was putting together the test project to validate before adding all of the fun and necessary code like error handling, which I didn't need when I was running a single program but which was necessary for the multiple versions. Which lead to:
private void DoSomething(string VariableID)
{
cmdExecuteThis.Parameters["@VariableID"].Value = VariableID;
try{
cmdExecuteThis.Connection.Open();
cmdExecuteThis.ExecuteNonQuery();
cmdExecuteThis.Connection.Close();
}
catch(Exception ex)
{
logTheError(ex.ToString());
}
}
And this worked up until the point where I encountered an error. Why? Well, that's because the next time through, the very first line of the try block threw an error, because I never closed the connection in the error. What I should have done was added:
finally
{
if(cmdExecuteThis.Connection.State != ConnectionState.Closed)
cmdExecuteThis.Connection.Close();
}
This will close the connection if it's been opened. Another method, for those into the Belt and Suspenders method is to check the connection state before modifying it. Which leads to the final format of:
private void DoSomething(string VariableID)
{
cmdExecuteThis.Parameters["@VariableID"].Value = VariableID;
try{
if(cmdExecuteThis.Connection.State != ConnectionState.Open)
cmdExecuteThis.Connection.Open();
cmdExecuteThis.ExecuteNonQuery();
if(cmdExecuteThis.Connection.State != ConnectionState.Closed)
cmdExecuteThis.Connection.Close();
}
catch(Exception ex)
{
logTheError(ex.ToString());
}
finally
{
if(cmdExecuteThis.Connection.State != ConnectionState.Closed)
cmdExecuteThis.Connection.Close();
}
}
The moral of the story? Short cuts are bad.