whytheq whytheq - 3 months ago 7
C# Question

Is this defensive code or are there possibilities that it could encouter problems?

I've got the following code which makes a connection to a db > runs a stored proc > and then moves on.

I believe it is easy to get db programming wrong so it is important to be defensive: is the following defensive? (or can it be improved?)

public int RunStoredProc()
{
SqlConnection conn = null;
SqlCommand dataCommand = null;
SqlParameter param = null;
int myOutputValue;

try
{
conn = new SqlConnection(ConfigurationManager.ConnectionStrings["IMS"].ConnectionString);
conn.Open();
dataCommand = conn.CreateCommand();
dataCommand.CommandType = CommandType.StoredProcedure;
dataCommand.CommandText = "pr_blahblah";
dataCommand.CommandTimeout = 200; //seconds
param = new SqlParameter();
param = dataCommand.Parameters.Add("@NumRowsReturned", SqlDbType.Int);
param.Direction = ParameterDirection.Output;
dataCommand.ExecuteNonQuery();
myOutputValue = (int)param.Value;

return myOutputValue;
}
catch (SqlException ex)
{
MessageBox.Show("Error:" + ex.Number.ToString(), "Error StoredProcedure");
return 0;
}
finally
{
if (conn != null)
{
conn.Close();
conn.Dispose();
}
}
}


CODE NOW LOOKS LIKE THE FOLLOWING

I've tried to use all the help offered by everyone and the above code has now been amended to the following which I hope is now sufficiently defensive:

public SqlConnection CreateConnection()
{
SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["IMS"].ConnectionString);
return conn;
}
public int RunStoredProc()
{
using (var conn = CreateConnection())
using (var dataCommand = conn.CreateCommand())
{
conn.Open();
dataCommand.CommandType = CommandType.StoredProcedure;
dataCommand.CommandText = "pr_BankingChargebacks";
dataCommand.CommandTimeout = 200; //5 minutes
SqlParameter param = new SqlParameter();
param = dataCommand.Parameters.Add("@NumRowsReturned", SqlDbType.Int);
param.Direction = ParameterDirection.Output;
dataCommand.ExecuteNonQuery();
int myOutputValue = (int)param.Value;

return myOutputValue;

}
}

Answer

Try using, well, the using construct for such things.

using(var conn = new SqlConnection(ConfigurationManager.ConnectionStrings["IMS"].ConnectionString)
{
}

Once you do that, I think you will be at the right level of "defense". Similarly try to do the same for anything that has to be disposed ( like the command)

Comments