Populating DataGrids

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.Connectio

• 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, 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> 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, to avoid duplicating code.

StackExchange Code Review Q#55906, answer score: 3