Verbesserte Code-Lesbarkeit für SQL-Befehle

Question

In einer meiner Klassen für eine Webanwendung, die ich entwickle, habe ich einige recht lange SQL-Abfragen.

Bei der Entwicklung einer dreistufigen Anwendung: Was sind die besten Verfahren, um diese Art von Code zu verbessern?

Dim dc As New SqlCommand("INSERT INTO Choices VALUES ('" + _ 
           SanitizeInput(strUser) + "', '" + _ 
           SanitizeInput(strFirstHalfDay) + "', '" + _ 
           SanitizeInput(strSecondHalfDay) + "', '" + _ 
           SanitizeInput(strFullDay) + "', " + _ 
           SanitizeInput(Convert.ToInt32(firstHalfPaid).ToString()) + ", " + _ 
           SanitizeInput(Convert.ToInt32(secondHalfPaid).ToString()) + ", " + _ 
           SanitizeInput(Convert.ToInt32(fullPaid).ToString()) + ")", cn) 

Haben Sie diese Art von Code betrachten als akzeptabel oder stinkende?

Answer 1

STOPP, tun Sie das nicht, verwenden Sie vorbereitete Stents, erhalten Sie Sicherheit und Lesbarkeit.

Verwenden Sie stattdessen:

Dim dc As New SqlCommand("INSERT INTO Choices VALUES (@User, @FirstHalfDay, @SecondHalfDay, @FullDay, @FirstHalfPaid, @SecondHalfPaid, @FullPaid'", cn) 
dc.Parameters.Add (new SqlParameter ("User", strUser)) 
dc.Parameters.Add (new SqlParameter ("FirstHalfDay", strFirstHalfDay)) 
dc.Parameters.Add (new SqlParameter ("SecondHalfDay", strSecondHalfDay)) 
dc.Parameters.Add (new SqlParameter ("FullDay", strFullDay)) 
dc.Parameters.Add (new SqlParameter ("FirstHalfPaid", firstHalfPaid)) 
dc.Parameters.Add (new SqlParameter ("SecondHalfPaid", secondHalfPaid)) 
dc.Parameters.Add (new SqlParameter ("FullPaid", fullPaid)) 

Answer 2

Stinky - das SanitizeInput für SQL-Injection mit ziemlicher Sicherheit gefährdet ist.

Ich würde einen parametrisierten SP (Code aus dem DB-Schema generiert, möglicherweise manuell von Hand nachgezwängt), mit einem .NET-Wrapper (auch aus dem DB-Schema generiert).

Answer 3

Direkt Umschreiben des in C# wäre:

SqlCommand dc = new SqlCommand(String.Format(@" 
    INSERT INTO Choices VALUES ('{0}', '{1}', '{2}', {3}, {4}, {5}) 
", SanitizeInput(strUser), SanitizeInput(strFirstHalfDay), SanitizeInput(strFullDay), SanitizeInput(Convert.ToInt32(firstHalfPaid).ToString()), SanitizeInput(Convert.ToInt32(secondHalfPaid).ToString()), SanitizeInput(Convert.ToInt32(fullPaid).ToString())), cn); 

Ich benutze string.format viel statt String-Verkettung. Es verbessert auch die Lesbarkeit. Ich bin nicht mit VB vertraut, sorry.

Ich stimme zu, dass die Verwendung von Parametern eine bessere Lösung ist.

Answer 4

Sie könnten eine Datenabstraktion von Drittanbietern wie Hibernate verwenden (auf Java - ich weiß nicht, was für .NET verfügbar ist).

Oder Sie könnten Ihre eigenen Hilfsklassen schreiben, die Syntax erlauben wie:

Dim insHelper As New InsertionHelper(conn, "Choices"); 

insHelper.appendValue(strUser); 
insHelper.appendValue(strFirstHalfDay); 
insHelper.appendValue(strSecondHalfDay); 
insHelper.appendValue(strFullDay); 
insHelper.appendValue(firstHalfPaid); // overloaded for different data types 
insHelper.appendValue(secondHalfPaid); 
insHelper.appendValue(fullPaid); 

insHelper.execute(); 

Sie könnten sogar Methode zur Mischung Verkettungs hinzufügen:

Dim insHelperAs New InsertionHelper(conn, "Choices"); 

insHelper 
    .appendValue(strUser) 
    .appendValue(strFirstHalfDay) 
    .appendValue(strSecondHalfDay) 
    .appendValue(strFullDay) 
    .appendValue(firstHalfPaid) 
    .appendValue(secondHalfPaid) 
    .appendValue(fullPaid) 
    .execute(); 

Answer 5

Dieser Ansatz ist für ein kleines Projekt in Ordnung, Jedoch sollten Sie für alles von bedeutender Größe entweder eine Bibliothek von Drittanbietern in Betracht ziehen, um Ihre Anfragen zu erstellen oder eigene zu entwickeln.

Das allgemeine Muster I geht so etwas verwenden: (. I Java-Syntax bin mit, weil das ist, was ich am meisten gewohnt bin)

Abfrage Vorlagen Eigenschaften

some.query=INSERT INTO Choices VALUES ('{0}', '{1}', '{2}', '{3}') 

Framework-Code

Datei

public class SqlUtil { 

    static Properties queries; 
    static { 
     queries = <LOAD QUERY PROPS> 
    } 

    public static SqlCommand createCommand(String propName, Object params ...) 
     throws SqlException 
    { 
     String cmd = queries.getString(propName) 

     if (propName == null || "".equals(propName)) 
      throw new SqlException("Query not found"); 

     int i=0; 
     for (Object param : params) { 
      cmd = cmd.replace("{" + i + "}", param) 
      i++; 
     } 

     return new SqlCommand(cmd); 
    } 

} 

Usage Code:

SqlCommand dc = SqlUtil.createCommand("some.query", SanitizeInput(strUser), SanitizeInput(strFirstHalfDay)) 

Answer 6

Ich würde dies für besser lesbar und sicherer halten, d.h.neigt weniger zu SQL Injection:

Dim dc As New SqlCommand("INSERT INTO Choices VALUES (@UserName, @FirstHalfDay, @SecondHalfDay, @FullDay, @FirstHalfPaid, @SecondHalfPaid, @FullPaid)", cn) 

dc.Parameters.AddWithValue("@UserName", strUsername) 
dc.Parameters.AddWithValue("@FirstHalfDay", strFirstHalfDay) 
dc.Parameters.AddWithValue("@SecondHalfDay", strSecondHalfDay) 
dc.Parameters.AddWithValue("@FullDay", strFullDay) 
dc.Parameters.AddWithValue("@FirstHalfPaid", Convert.ToInt32(firstHalfPaid)) 
dc.Parameters.AddWithValue("@SecondHalfPaid", Convert.ToInt32(secondHalfPaid)) 
dc.Parameters.AddWithValue("@FullPaid", Convert.ToInt32(fullPaid)) 

Wenn dies für ausgewählte Anweisungen zu benutzen Sie den zusätzlichen Vorteil erhalten, wenn Sie SQL Server verwenden, die als es die gleiche Zeichenfolge wird in der Lage, den Abfrageplan cachen nur paramterised. Wenn Sie nicht mit SQL Server arbeiten, sind möglicherweise geringfügige Änderungen erforderlich. Sie erhalten auch einen anderen Vorteil für die kostenlose, die etwas bessere Leistung durch weniger String-Verkettung ist.

Ich würde persönlich die Spalten nach dem Choices-Tabellennamen hinzufügen, die weniger anfällig für Brüche wären, wenn neue Spalten in die Datenbanktabelle aufgenommen würden (unter Annahme von Standardwerten).

Answer 7

Während ich denke, Ihr Code ist wunderbar, ich habe eine nicht erschöpfende Liste von möglichen Verbesserungen:

• Sie schreiben SQL stattdessen eine Mapping-Technologie verwenden (NHibernate, Linq to SQL, EF, SubSonic, LLBLGenPro, etc.)
• Zugegeben, dass Sie SQL schreiben, verwenden Sie keine Parameter. Zumindest Sie reinigen die Eingabe korrekt, während Sie String-Verkettung machen ... hoffentlich ....
• Zugegeben, dass Sie SQL ohne Parameter schreiben, verwenden Sie nicht String.Format, um Ihre String-Erstellung und String- Formatierung.
• Ihre Variablennamen sind unordentlich. Sie benennen Ihre Variablen mit einer ungeprüften ungarischen Notationskonvention. Ungarische Notation war ursprünglich beabsichtigt, um Ihre Variablen Namen zu vermitteln, wie diese Variablen sind verwendet, um ihre Typen nicht zu vermitteln.
• Sie codieren die Implementierung, nicht die Schnittstelle. Ihre Verbindungsvariable sollte IDbConnection eingegeben werden und Ihre Befehlsvariable sollte IDbCommand eingegeben werden. Nicht SqlConnection und SqlCommand. Verwenden Sie IDbConnection.CreateCommand().
• SanitizeInput sollte umbenannt werden EscapeStringForSQL oder etwas, das seine tatsächliche Absicht angibt. Sie müssen Zahlen nicht entkommen.

Answer 8

Machen Sie den SQL-Befehl vor dem Aufruf des Konstruktors zu einer separaten Zeichenfolge. Es wird einfacher zu lesen sein.

Statt:

Dim dc As New SqlCommand("INSERT INTO SomeTable....", 
         Arg(blah blah blah)) 

tun:

Dim sqlstr = 
"INSERT INTO SomeTable...." 

Dim dc As New SqlCommand(sqlstr, Arg(blah blah blah)) 

(Verzeihen Sie meine nicht die richtige VB Syntax zu kennen)

Die Sache ist die, dass die Zeichenkette in die SqlCommand Gießen () Konstrukteur bringt gerade eine Menge Zeug in einen großen Befehl, den jemand später in seinem Kopf auseinander brechen muss.

Auch wenn Sie die SQL Sie ausführen, während das Debuggen drucken möchten, wird es viel einfacher sein, in einem einfachen

Print "Now executing: " + sqlstr 

zu werfen Sorgen Sie sich nicht, dass Sie ein „extra“ sind die Schaffung Variable. Der Computer macht nichts.