4
\$\begingroup\$

I was just writing some code the other day, and I found myself writing something I am not sure whether it is OK to do it like this, or not.

Let me give an example of what this is:

I have this Method:

 public static string STOLENVersion()
 {
 string version = string.Empty;
 try
 {
 StringBuilder sb = new StringBuilder();
 sb.AppendLine("SELECT TOP 1");
 sb.AppendLine(" [WebsiteVersion]");
 sb.AppendLine("FROM [WebsiteDefaults]");
 using (SqlCommand cmd = new SqlCommand(sb.ToString(), NewSQLConnection))
 {
 cmd.Connection.Open();
 version = cmd.ExecuteScalar().ToString();
 cmd.Connection.Dispose();
 }
 }
 catch (Exception)
 {
 throw;
 }
 return "Version: " + version;
 }

And I have this property:

public static SqlConnection NewSQLConnection
 {
 get
 {
 return new SqlConnection(ConfigurationManager.ConnectionStrings["DebugDB"].ConnectionString);
 }
 }

I am disposing the SqlCommand's connection inside its using block, and this method can be called multiple times (maybe like 10 times every minute, as users log on to the website, as an example)

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jun 1, 2016 at 17:27
\$\endgroup\$
2
  • \$\begingroup\$ Is this code being run from a utility program, or your actual website? \$\endgroup\$ Commented Jun 2, 2016 at 8:53
  • \$\begingroup\$ From my actual website :) \$\endgroup\$ Commented Jun 8, 2016 at 20:53

2 Answers 2

3
\$\begingroup\$
  • The try-catch-throw with no error handling is completely pointless. I would just remove it and let the exception bubble up the call stack.

  • There is no need to call connection.Dispose() since the using () { ... } block will do that automatically

  • A StringBuilder object is like hitting a thumbtack with a sledge hammer in this case. You can prefix a string literal with the @ symbol to allow newline characters for better readibility:

    string sql = @"SELECT TOP 1
     [WebsiteVersion]
     FROM [WebsiteDefaults]";
    

    Or since this is such a simple SQL statement, a one-liner is appropriate as well:

    string sql = "SELECT TOP 1 [WebsiteVersion] FROM [WebsiteDefaults]";
    
answered Jun 1, 2016 at 22:06
\$\endgroup\$
4
  • \$\begingroup\$ Or you can use string concatenation string sql = "SELECT TOP 1"+" [WebsiteVersion] "+" FROM [WebsiteDefaults]"; and compiller is quite smart to perform it at compile time. (sadly that comments on this site don't allow line-breaks) \$\endgroup\$ Commented Jun 1, 2016 at 22:47
  • \$\begingroup\$ So, disposing the "SqlCommand" will also dispose the SqlConnection? \$\endgroup\$ Commented Jun 2, 2016 at 4:50
  • \$\begingroup\$ That's a good question. To be honest, I always create the SqlConnection object in its own using statement to ensure it gets garbage collected properly. @Greg already mentioned that in his answer. \$\endgroup\$ Commented Jun 2, 2016 at 12:11
  • \$\begingroup\$ I see. Nah I'll just add the SqlConnection with a using block then :). Thanx \$\endgroup\$ Commented Jun 2, 2016 at 14:07
2
\$\begingroup\$

Normally, you would see the following:

using(var connection = new SqlConnection(dbConnection))
 using(var command = new SqlCommand(query, connection))
 {
 }

The reason, you wouldn't have to worry about forgetting to close the connection. Which could introduce congruency issues.

However, your approach is fine assuming other developers don't open a connection that is already open, or close a connection that isn't closed already.

answered Jun 1, 2016 at 20:44
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.