patterncsharpMinor
Fetching the website version number from the database
Viewed 0 times
websitethenumberversiondatabasefetchingfrom
Problem
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:
And I have this property:
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)
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)
Solution
-
The
-
There is no need to call
-
A StringBuilder object is like hitting a thumbtack with a sledge hammer in this case. You can prefix a string literal with the
Or since this is such a simple SQL statement, a one-liner is appropriate as well:
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]";Code Snippets
string sql = @"SELECT TOP 1
[WebsiteVersion]
FROM [WebsiteDefaults]";string sql = "SELECT TOP 1 [WebsiteVersion] FROM [WebsiteDefaults]";Context
StackExchange Code Review Q#129870, answer score: 3
Revisions (0)
No revisions yet.