EDIT Based on the great comment of @d347hm4n stating
In the above code, in all cases, there will be a comma on the end that needs to be removed. So removing the last character from the string builder is sufficient. If the sql generated does change in the future, using a TrimEnd instead gives you the extra intelligence to not remove the comma on the end if it's not there.
I changed the former removing of the comma from
sb.Length -= 1;
string query = sb.ToString();
to
string query = sb.ToString().TrimEnd(',');
Applying the mentioned points lead to
public static Dictionary<string, bool> GetPermissions(IEnumerable<string> tables, IEnumerable<string> permissionTypes)
{
if (!tables.Any() || !permissionTypes.Any())
{
return new Dictionary<string, bool>();
}
StringBuilder sb = new StringBuilder(1024);
sb.Append("SELECT ");
foreach (string table in tables)
{
foreach (string perm in permissionTypes)
{
sb.AppendFormat("HAS_PERMS_BY_NAME('{0}','OBJECT','{1}') AS {0}_{1},", table, perm);
}
}
sb.Length -= 1; // remove the last comma
string query = sb.ToString().TrimEnd(',');
Dictionary<string, bool> permissions = new Dictionary<string, bool>();
using (SqlConnection sql = new SqlConnection(ConnectionString))
using (SqlCommand cmd = new SqlCommand(query, sql))
{
sql.Open();
using (SqlDataReader data = cmd.ExecuteReader())
{
data.Read();
for (int i = 0; i < data.FieldCount; i++)
{
permissions.Add(data.GetName(i), data[i].Equals(1));
}
}
}
return permissions;
}
Applying the mentioned points lead to
public static Dictionary<string, bool> GetPermissions(IEnumerable<string> tables, IEnumerable<string> permissionTypes)
{
if (!tables.Any() || !permissionTypes.Any())
{
return new Dictionary<string, bool>();
}
StringBuilder sb = new StringBuilder(1024);
sb.Append("SELECT ");
foreach (string table in tables)
{
foreach (string perm in permissionTypes)
{
sb.AppendFormat("HAS_PERMS_BY_NAME('{0}','OBJECT','{1}') AS {0}_{1},", table, perm);
}
}
sb.Length -= 1; // remove the last comma
string query = sb.ToString();
Dictionary<string, bool> permissions = new Dictionary<string, bool>();
using (SqlConnection sql = new SqlConnection(ConnectionString))
using (SqlCommand cmd = new SqlCommand(query, sql))
{
sql.Open();
using (SqlDataReader data = cmd.ExecuteReader())
{
data.Read();
for (int i = 0; i < data.FieldCount; i++)
{
permissions.Add(data.GetName(i), data[i].Equals(1));
}
}
}
return permissions;
}
EDIT Based on the great comment of @d347hm4n stating
In the above code, in all cases, there will be a comma on the end that needs to be removed. So removing the last character from the string builder is sufficient. If the sql generated does change in the future, using a TrimEnd instead gives you the extra intelligence to not remove the comma on the end if it's not there.
I changed the former removing of the comma from
sb.Length -= 1;
string query = sb.ToString();
to
string query = sb.ToString().TrimEnd(',');
Applying the mentioned points lead to
public static Dictionary<string, bool> GetPermissions(IEnumerable<string> tables, IEnumerable<string> permissionTypes)
{
if (!tables.Any() || !permissionTypes.Any())
{
return new Dictionary<string, bool>();
}
StringBuilder sb = new StringBuilder(1024);
sb.Append("SELECT ");
foreach (string table in tables)
{
foreach (string perm in permissionTypes)
{
sb.AppendFormat("HAS_PERMS_BY_NAME('{0}','OBJECT','{1}') AS {0}_{1},", table, perm);
}
}
string query = sb.ToString().TrimEnd(',');
Dictionary<string, bool> permissions = new Dictionary<string, bool>();
using (SqlConnection sql = new SqlConnection(ConnectionString))
using (SqlCommand cmd = new SqlCommand(query, sql))
{
sql.Open();
using (SqlDataReader data = cmd.ExecuteReader())
{
data.Read();
for (int i = 0; i < data.FieldCount; i++)
{
permissions.Add(data.GetName(i), data[i].Equals(1));
}
}
}
return permissions;
}
So, any tips that reduce lines of code..
As a start you can remove this useless try..catch because you are only rethrowing the exception which is senseless.
Using += with strings in a loop is a bad habit because each time a new string will be created so better use a StringBuilder.
Speeking about the composition of the query variable you don't need to add new lines and tabs in it. You won't show it, you only execute it as a sql query.
In its current state this method is not flexible enough. Each time you want to query different tables or need only specific permissions you need to change this method.
By passing IEnumerable<string> tables and IEnumerable<string> permissionType you can beautify your method and add a lot flexibility to it.
Let us talk about style. If it comes to style one should stick to a choosen style.
If you choose to use braces {} for loops you should always use them. Mixing styles is a bad habit, don't do it.
Method scoped variables should be named using camelCase casing. PascalCase casing is used for class-, method- and propertynames. So Dictionary<string,bool> Permissions -> Dictionary<string,bool> permissions.
Applying the mentioned points lead to
public static Dictionary<string, bool> GetPermissions(IEnumerable<string> tables, IEnumerable<string> permissionTypes)
{
if (!tables.Any() || !permissionTypes.Any())
{
return new Dictionary<string, bool>();
}
StringBuilder sb = new StringBuilder(1024);
sb.Append("SELECT ");
foreach (string table in tables)
{
foreach (string perm in permissionTypes)
{
sb.AppendFormat("HAS_PERMS_BY_NAME('{0}','OBJECT','{1}') AS {0}_{1},", table, perm);
}
}
sb.Length -= 1; // remove the last comma
string query = sb.ToString();
Dictionary<string, bool> permissions = new Dictionary<string, bool>();
using (SqlConnection sql = new SqlConnection(ConnectionString))
using (SqlCommand cmd = new SqlCommand(query, sql))
{
sql.Open();
using (SqlDataReader data = cmd.ExecuteReader())
{
data.Read();
for (int i = 0; i < data.FieldCount; i++)
{
permissions.Add(data.GetName(i), data[i].Equals(1));
}
}
}
return permissions;
}
and can be then called like this
IEnumerable<string> tables = new string[] { "appearance", "appearanceSpecs", "cutTemplates", "cutVinyl" };
IEnumerable<string> permissionTypes = new string[] { "INSERT", "UPDATE", "DELETE" };
Dictionary<string, bool> permissions = GetPermissions(tables, permissionTypes);