C＃コード/ハードコードの最適化

Question

最初に、私はコーダーではないことを言及したいと思います。基本的に、私は最初からC＃を学んだり、本当に気にしません。

ここでは、私のコードが達成しようとしていることについての少しの背景があります。

データベース名にカスケードDDL1があります。ユーザーがデータベース名を選択すると、日付範囲（「some」パラメーターで異なる照会を実行する）に対してDDL2を選択する必要があります。

実行されるクエリには、異なるデータベースの2つのテーブルを結合することが含まれます（これは私が持っている全体的な問題だと思います）。

私はコードを動作させることができますが、それは本当に退屈なように思えます。ハードコーディングは、必要な場合を除いて、ぶつかっていると確信しています。私は少しの研究を行い、ストアドプロシージャでデータベース名をパラメータ化するために動的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を再編成して、そのステータスが1つの場所にのみ表示されるようにすることができます。また、if文の大きなブロックの代わりにswitch文を使用する方法も検討する必要があります。

ハードコーディングについては、完了したステータスはちょっと奇妙です。 'done'になる場合は、常にparamの代わりにselect文のその部分を作成してください。あなたのようなインラインSQLを書くことは、必ずしも悪いとは限りません。あなたは良いSQLインジェクション攻撃を避けるためにあなたのクエリをパラメータ化しています。

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); 

は再びあなたはおそらく内部でこれらデシベル変数を割り当てる移動する必要がありますあなたのブロックとは終わりに向かってselectStatement宣言を移動した場合。