Populating DataGrids

Question

I'm building an in house query tool. The main form has a drop down which allows the user to select how they want to find scripts, and a drop down for what server they want to run the query on. There is also a text box to allow the user to populate a specific value which can be left blank. I think I did pretty well here. This is my first desktop WinForms app.

What Happens When The Query Is Run

private void QueryButton_Click(object sender, EventArgs e)
{
    SqlConnection connection = GetSqlConnection();
    string value = this.ValueTextBox.Text;
    TableAdapters.FindScript dataSource = new TableAdapters.FindScript();

    switch (this.FindByComboBox.SelectedItem.ToString())
    {
        case "Find Scripts By Target VDN":
            this.FindScriptsDataGrid.DataSource = dataSource.FillByTargetVDN(connection, value).Tables[0];
            break;
        case "Find Scripts By Skill Group":
            this.FindScriptsDataGrid.DataSource = dataSource.FillBySkillGroup(connection, value).Tables[0];
            break;
        case "Find Scripts By Translation Route Pool":
            this.FindScriptsDataGrid.DataSource = dataSource.FillByTranslationRoutePool(connection, value).Tables[0];
            break;
        case "Find Scripts By Name":
            this.FindScriptsDataGrid.DataSource = dataSource.FillByName(connection, value).Tables[0];
            break;
        case "Find Scripts By Label":
            this.FindScriptsDataGrid.DataSource = dataSource.FillByLabel(connection, value).Tables[0];
            break;
    }

    foreach (DataGridViewRow row in this.FindScriptsDataGrid.Rows)
    {
        row.HeaderCell.Value = (row.Index + 1).ToString();
    }
}

// SERVER NAMES CENSORED IN THIS POST FOR SECURITY
private SqlConnection GetSqlConnection()
{
    SqlConnection connection = new SqlConnection();
    switch (this.ServerComboBox.SelectedItem.ToString())
    {
        case "SERVER1":
            connection.ConnectionString = ConfigurationManager.ConnectionStrings["csS01"].ConnectionString;
            break;
        case "SERVER2":
            connection.ConnectionString = ConfigurationManager.ConnectionStrings["csS02"].ConnectionString;
            break;
    }
    return connection;
}

The Queries Being Called

class TableAdapters
{
    public class FindScript
    {
        string conditional = string.Empty;
        string query = string.Empty;

        DataSet Fill(SqlConnection connection, string query)
        {
            SqlDataAdapter adapter = new SqlDataAdapter(query, connection);
            DataSet dataSet = new DataSet();
            dataSet.Tables.Add(new DataTable());
            adapter.Fill(dataSet.Tables[0]);
            return dataSet;
        }

        public DataSet FillByTargetVDN(SqlConnection connection, string parameter)
        {
            if (parameter.Length > 0)
            {
                conditional = "AND svce.PeripheralNumber= " + parameter + " ";
            }
            query = "SELECT svce.PeripheralNumber AS Service, "
                + "ms.EnterpriseName AS [Script Name], "
                + "p.EnterpriseName AS Peripheral, " 
                + "sg.PeripheralNumber AS [Skill Mapping], " 
                + "s.Version AS [Latest Version], "
                + "s.DateTime AS Created, " 
                + "s.Author " 
                + "FROM Script_Cross_Reference scr "
                + "LEFT OUTER JOIN Script s ON s.ScriptID = scr.ScriptID "
                + "LEFT OUTER JOIN Master_Script ms ON s.MasterScriptID = ms.MasterScriptID "
                + "LEFT OUTER JOIN Service svce ON svce.SkillTargetID = scr.ForeignKey "
                + "LEFT OUTER JOIN Peripheral p ON svce.PeripheralID = p.PeripheralID "
                + "LEFT OUTER JOIN Service_Member sm ON svce.SkillTargetID = sm.ServiceSkillTargetID " 
                + "LEFT OUTER JOIN Skill_Group sg ON sg.SkillTargetID = sm.SkillGroupSkillTargetID " 
                + "WHERE s.Version = ms.CurrentVersion AND scr.TargetType = 1 "
                + conditional
                + "ORDER BY svce.PeripheralNumber, ms.EnterpriseName, p.EnterpriseName";
            return Fill(connection, query);
        }

        public DataSet FillBySkillGroup(SqlConnection connection, string parameter)
        {
            if (parameter.Length > 0)
            {
                conditional = "AND sg.PeripheralNumber = " + parameter + " ";
            }
            query = "SELECT DISTINCT(sg.PeripheralNumber) AS Skill, " 
                + "ms.EnterpriseName AS [Script Name], " 
                + "s.Version AS [Latest Version], " 
                + "s.DateTime AS Created, " 
                + "s.Author " 
                + "FROM Script_Cross_Reference scr "
                + "LEFT OUTER JOIN Script s ON s.ScriptID = scr.ScriptID "
                + "LEFT OUTER JOIN Master_Script ms ON s.MasterScriptID = ms.MasterScriptID "
                + "LEFT OUTER JOIN Skill_Group sg ON sg.SkillTargetID = scr.ForeignKey " 
                + "WHERE s.Version = ms.CurrentVersion AND scr.TargetType = 2 "
                + conditional 
                + "ORDER BY sg.PeripheralNumber, ms.EnterpriseName";
            return Fill(connection, query);
        }

        public DataSet FillByTranslationRoutePool(SqlConnection connection, string parameter)
        {
            if (parameter.Length > 0)
            {
                conditional = "AND (tr.EnterpriseName like ('" + parameter + "') "
                    + "OR tr.EnterpriseName like ('" + parameter.ToUpper() + "') "
                    + "OR tr.EnterpriseName like ('" + parameter.ToLower() + "')) ";
            }
            query = "SELECT DISTINCT(tr.EnterpriseName) AS Translation_Route, " 
                + "ms.EnterpriseName AS [Script Name], " 
                + "s.Version AS [Latest Version], " 
                + "s.DateTime AS Created, " 
                + "s.Author " 
                + "FROM Script_Cross_Reference scr " 
                + "LEFT OUTER JOIN Script s ON s.ScriptID = scr.ScriptID " 
                + "LEFT OUTER JOIN Master_Script ms ON s.MasterScriptID = ms.MasterScriptID " 
                + "LEFT OUTER JOIN Translation_Route tr ON scr.ForeignKey = tr.SkillTargetID " 
                + "WHERE s.Version = ms.CurrentVersion AND scr.TargetType = 4 "
                + conditional 
                + "ORDER BY tr.EnterpriseName, ms.EnterpriseName";
            return Fill(connection, query);
        }

        public DataSet FillByName(SqlConnection connection, string parameter)
        {
            if (parameter.Length > 0)
            {
                conditional = "AND (ms.EnterpriseName LIKE ('" + parameter + "') "
                    + "OR ms.EnterpriseName LIKE ('" + parameter.ToUpper() + "') "
                    + "OR ms.EnterpriseName LIKE ('" + parameter.ToLower() + "')) ";
            }
            query = "SELECT ms.EnterpriseName AS [Script Name], "
                + "s.Author, "
                + "s.Version, "
                + "s.Valid, "
                + "s.DateTime AS Created "
                + "FROM Script s "
                + "LEFT OUTER JOIN Master_Script ms ON s.MasterScriptID = ms.MasterScriptID "
                + "WHERE s.Version = ms.CurrentVersion "
                + conditional
                + "ORDER BY ms.EnterpriseName";
            return Fill(connection, query);
        }

        public DataSet FillByLabel(SqlConnection connection, string parameter)
        {
            if (parameter.Length > 0)
            {
                conditional = "AND l.Label='" + parameter + "' ";
            }
            query = "SELECT DISTINCT l.Label AS Label, "
                + "ms.EnterpriseName AS [Script Name], "
                + "s.Version AS [Latest Version], "
                + "s.DateTime AS Created, "
                + "s.Author "
                + "FROM Script_Cross_Reference scr "
                + "LEFT OUTER JOIN Script s ON s.ScriptID = scr.ScriptID "
                + "LEFT OUTER JOIN Master_Script ms ON s.MasterScriptID = ms.MasterScriptID "
                + "LEFT OUTER JOIN Label l ON l.LabelID = scr.ForeignKey "
                + "WHERE s.Version = ms.CurrentVersion AND scr.TargetType = 19 "
                + conditional
                + "ORDER BY l.Label, ms.EnterpriseName";
            return Fill(connection, query);
        }
    }
}

Now I already have some concerns about the whole conditional thing in the TableAdapters class which I address here. Other than that are there other areas of improvement that you could point out to me?

Answer 1

• I assume it's your style to refer to class members using "this.". That's not the usual way to do things but it is acceptable.
• It also seems to be your style to capitalize the names of the UI elements. Again, not the usual style but not horrible (it does confuse the syntax coloration here though).
• You really need to separate the UI and the business logic. The way things stand now, it would be impossible to drive the program via an API or a batch process. Especially notable since that's usually the best way to test it.
• I don't believe it's ideal for conditional to be a class member of FindScript. There's no reason it needs to persist.
• And I'm pretty sure all of the FindScript class could get replaced by a Dictionary<String,String>, maybe with some "{0}" in there so you can use String.Format().

• That big switch statement in QueryButton_Click() could be refactored into a dictionary.

  For instance:

  private static TableAdapters.FindScript dataSource = new TableAdapters.FindScript();
  
  private static readonly Dictionary<String, Func<SqlConnection,String,DataSet>> switchReplacement = new Dictionary()
  {
      {"Find Scripts By Target VDN", dataSource.FillByTargetVDN},
      {"Find Scripts By Skill Group", dataSource.FillBySkillGroup},
      {"Find Scripts By Translation Route Pool", dataSource.FillByTranslationRoutePool},
      {"Find Scripts By Name", dataSource.FillByName},
      {"Find Scripts By Label", dataSource.FillByLabel}
  };
  

  And then in the function, you can avoid repeating yourself:

  SqlConnection connection = GetSqlConnection();
  string value = this.ValueTextBox.Text;
  this.FindScriptsDataGrid.DataSource = switchReplacement[this.FindByComboBox.SelectedItem.ToString()](connection, value).Tables[0];
  

  Similarly, the switch in GetSqlConnection() could be a Dictionary<String,String>, to avoid duplicating code.