Оптимизация кода C#/жесткого кодирования

Question

Во-первых, я хочу упомянуть, что я не кодер. В принципе, я застрял в обучении C# с нуля, и я не против.

Вот небольшой пример того, что мой код пытается выполнить.

У меня есть каскадный DDL1 для имени базы данных. Когда пользователь выбирает имя базы данных, им нужно будет выбрать DDL2 для диапазона дат (который выполняет разные запросы с «некоторыми» параметрами).

Запрос, который он запускает, включает объединение двух таблиц из разных баз данных (я думаю, что это общая проблема, которая у меня есть).

Я могу заставить код работать, но он кажется действительно утомительным, и я уверен, что жесткое кодирование неодобрительно, если это не необходимо. Я провел несколько исследований, и похоже, что мне нужно будет использовать динамический SQL для параметризации имени базы данных в хранимой процедуре, которую я предпочитаю не использовать.

Итак, в резюме. Для каждого имени базы данных и каждого диапазона дат мне нужно будет жестко записать их отдельно. Это будет в общей сложности 4 блока кода для каждой базы данных. Если все данные существовали в 1 таблице, у меня не было бы этой проблемы.

Опять же, я довольно новый и в значительной степени новичок. Пожалуйста, дайте мне знать, если возможно оптимизировать код и сделать его более эффектным, потому что на данный момент он выглядит как плохое кодирование.

код приведен ниже, спасибо:

protected void DropDownList1_SelectedIndexChanged(object sender, EventArgs e) 
{ 

{ 
    DataTable dt = new DataTable(); 
    SqlDataAdapter Adpt; 

    if (DropDownList1.SelectedValue == string.Empty & DropDownList2.SelectedValue == string.Empty) 
    { 
    } 

    if (DropDownList1.SelectedItem.Text == "ytd" & DropDownList2.SelectedValue == "db1") 
     using (SqlConnection con = new SqlConnection(ConfigurationManager.ConnectionStrings["db1ConnectionString"].ConnectionString)) 
     { 
      SqlCommand cmd = new SqlCommand("select sum(column1) from table1 inner join db2.dbo.table2 on db2.dbo.table2.ID = db1.dbo.table3.id where somecolumn = @somecolumn and Date <= GetDate() AND YEAR(Date) = year(GetDate()) AND Status = @Status", con); 
      cmd.Parameters.AddWithValue("@somecolumn", DropDownList2.SelectedValue); 
      cmd.Parameters.AddWithValue("@status", "done"); 
      Adpt = new SqlDataAdapter(cmd); 
      new SqlDataAdapter(cmd).Fill(dt); 
     } 

    if (DropDownList1.SelectedItem.Text == "yesterday" & DropDownList2.SelectedValue == "db1") 
     using (SqlConnection con = new SqlConnection(ConfigurationManager.ConnectionStrings["db1ConnectionString"].ConnectionString)) 
     { 
      SqlCommand cmd = new SqlCommand("select sum(column1) from table1 inner join db2.dbo.table2 on db2.dbo.table2.ID = db1.dbo.table3.id where somecolumn = @somecolumn and Date >= DATEADD(DAY, DATEDIFF(DAY, 1, GETDATE()), 0) AND Date <= DATEADD(DAY, DATEDIFF(DAY, 0, GETDATE()), 0) AND Status = @Status", con); 
      cmd.Parameters.AddWithValue("@somecolumn", DropDownList2.SelectedValue); 
      cmd.Parameters.AddWithValue("@status", "done"); 
      Adpt = new SqlDataAdapter(cmd); 
      new SqlDataAdapter(cmd).Fill(dt); 

    //if same code, different database 
     { 
     } 

    //if same code, different database 
     { 
     } 

    //else if same code, different database   
     { 
     } 

    GridView1.DataSource = dt; 
    GridView1.DataBind(); 

} 


protected void DropDownList2_SelectedIndexChanged(object sender, EventArgs e) 
{ 
    { 
     if (DropDownList2.SelectedIndex == 0) 
     { 
      DropDownList1.SelectedIndex = 0; 
      DropDownList1.Enabled = false; 
     } 
     else 
     { 
      DropDownList1.Enabled = true; 
      DropDownList1.SelectedIndex = 0; 
     } 

    } 
} 

}

Answer 1

Ключ к хорошему программированию является удаление избыточности, когда это возможно. Если большая часть вашего кода совпадает или очень похожа на то, как не повторять себя.

Например быстрый рефакторинга кода будет выглядеть примерно так:

var selectStatement = "select sum(column1) from table1 inner join db2.dbo.table2 on db2.dbo.table2.ID = db1.dbo.table3.id where somecolumn = @somecolumn " 
string whereClause; 

if (DropDownList2.SelectedValue == "db1") 
{ 
    if (DropDownList1.SelectedItem.Text == "ytd") 
    { 
     whereClause = "and Date <= GetDate() AND YEAR(Date) = year(GetDate()) AND Status = @Status"; 
    } 
    else if (DropDownList1.SelectedItem.Text == "yesterday") 
    { 
     whereClause = "and Date >= DATEADD(DAY, DATEDIFF(DAY, 1, GETDATE()), 0) AND Date <= DATEADD(DAY, DATEDIFF(DAY, 0, GETDATE()), 0) AND Status = @Status" 
    } 
} 

//etc 

SqlCommand cmd = new SqlCommand(selectStatement + whereClause, con); 
cmd.Parameters.AddWithValue("@somecolumn", DropDownList2.SelectedValue); 
cmd.Parameters.AddWithValue("@status", "done"); 
Adpt = new SqlDataAdapter(cmd); 
new SqlDataAdapter(cmd).Fill(dt); 

GridView1.DataSource = dt; 
GridView1.DataBind(); 

Существует еще больше возможностей для улучшения, хотя. Вы можете реорганизовать whereClause, чтобы статус отображался только в одном месте. Вы также должны изучить использование оператора switch вместо большого блока операторов if.

Что касается жесткого кодирования. Выполненное состояние немного странно. Если он будет «сделан», всегда просто делайте эту часть выражения select вместо параметра. Написание встроенного sql-файла, как и у вас, не обязательно плохо. У вас есть свои параметры, чтобы избежать атак SQL Injection, которые хороши.

EDIT: Один из способов идти об изменении БД является использование String.Format, который заменит теги {#} с переменным этим индексом.

var sourceDb = "db1"; 
var targetDb = "db2"; 
var selectStatement = string.Format("select sum(column1) from table1 inner join {0}.dbo.table2 on {0}.dbo.table2.ID = {1}.dbo.table3.id where somecolumn = @somecolumn ", targetDb, sourceDb); 

Глядя на ваш код еще раз вам, возможно, придется переместить присвоить эти DB переменные внутри вашего, если блоки и переместить декларацию SelectStatement к концу.