Optimieren von C# -Code/Hard-Codierung

Question

Zunächst möchte ich erwähnen, dass ich kein Coder bin. Im Grunde genommen habe ich gelernt, C# von Grund auf zu lernen, was mir eigentlich nichts ausmacht.

Hier ist ein kleiner Hintergrund dessen, was mein Code versucht zu erreichen.

Ich habe eine kaskadierende DDL1 für den Datenbanknamen. Wenn ein Benutzer den Datenbanknamen auswählt, müsste er DDL2 für den Datumsbereich auswählen (der verschiedene Abfragen mit "einigen" Parametern ausführt).

Die Abfrage, die ausgeführt wird, umfasst die Verbindung von 2 Tabellen, die aus verschiedenen Datenbanken stammen (ich denke, das ist das allgemeine Problem, das ich habe).

Ich kann den Code zu arbeiten, aber es scheint wirklich langweilig und ich bin sicher, dass harte Codierung ist verpönt, wenn es notwendig ist. Ich habe ein wenig Nachforschungen angestellt, und es sieht so aus, als müsste ich dynamisches SQL verwenden, um den Datenbanknamen in einer gespeicherten Prozedur zu parametrisieren, die ich lieber nicht verwende.

Also, zusammenfassend. Für jeden Datenbanknamen und jeden Datumsbereich müsste ich sie separat hart codieren. Das wären insgesamt 4 Codeblöcke für jede Datenbank. Wenn die Daten alle in einer Tabelle vorhanden wären, hätte ich dieses Problem nicht.

Wieder bin ich ziemlich neu und ziemlich ein Anfänger. Bitte lassen Sie mich wissen, ob es möglich ist, den Code zu optimieren und effizienter zu machen, denn im Moment sieht es für mich wie eine schlechte Kodierung aus.

Der folgende Code versehen ist, dank:

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

Der Schlüssel für eine gute Programmierung ist das Entfernen Redundanzen, wenn möglich. Wenn der Großteil Ihres Codes identisch oder sehr ähnlich ist, sollten Sie sich nicht wiederholen.

Zum Beispiel eine schnelle Umgestaltung des Codes würde wie folgt aussehen:

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

Es obwohl noch Raum für Verbesserungen. Sie können den whereClause so reorganisieren, dass der Status nur an einer Stelle erscheint. Sie sollten auch eine switch-Anweisung anstelle des großen Blocks von if-Anweisungen verwenden.

Wie für die harte Codierung. Der Status ist etwas seltsam. Wenn es "erledigt" wird, mache immer nur den Teil der Select-Anweisung anstelle eines Parameters. Inline-SQL zu schreiben, so wie du bist, ist nicht unbedingt schlecht. Sie haben Ihre Abfragen parametrisiert, um SQL Injection-Angriffe zu vermeiden, was gut ist.

EDIT: Ein Weg zu gehen über die db Wechsel ist String.Format zu verwenden, die die {#} Tags mit den Variablen dieses Index ersetzen wird.

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

auf den Code der Suche wieder werden Sie wahrscheinlich diese db Variablen in Ihrem wenn Blöcke und bewegen Sie die SELECT-Deklaration gegen Ende zuweisen bewegen müssen.