Mejorando la legibilidad del código para comandos SQL

Question

En una de mis clases para una aplicación web que estoy desarrollando, tengo algunas consultas SQL razonablemente largas.

Al desarrollar una aplicación de tres niveles, ¿cuáles son las mejores prácticas para hacer que este tipo de código sea más ordenado?

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) 

¿Considera que este tipo de código es aceptable o apestoso?

Answer 1

STOP, no hagas eso, usa staments preparados, obtendrás seguridad y legibilidad.

Use este lugar:

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 - SanitizeInput que es casi seguro vulnerable a la inyección de SQL.

Usaría un SP parametrizado (código generado a partir del esquema DB, posiblemente ajustado manualmente), con un contenedor .NET (también generado a partir del esquema DB).

Answer 3

reescritura directa de que en C# sería:

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

utilizo string.format un montón en lugar de la concatenación de cadenas. Mejora la legibilidad también. No estoy familiarizado con VB, lo siento.

Acepto que el uso de parámetros es una mejor solución.

Answer 4

Puede utilizar una abstracción de datos de terceros como Hibernate (en Java - No sé qué hay disponible para .NET).

O puede escribir sus propias clases de ayuda que permiten sintaxis como:

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

Incluso se puede añadir el método de encadenamiento a la mezcla:

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

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

Answer 5

Este enfoque está muy bien para un pequeño proyecto, Sin embargo, para cualquier cosa de tamaño significativo, debe considerar una biblioteca de terceros para construir sus consultas o desarrollar la suya propia.

El patrón general que utilizo es algo como esto: (. Estoy usando la sintaxis de Java, porque eso es lo que estoy más acostumbrado a) Propiedades

plantilla de consulta

Archivo

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

Código Marco

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

} 

Código de Uso:

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

Answer 6

Considero que esto es más legible y más seguro, es decir,menos propenso a la inyección de SQL:

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

Al hacerlo por sentencias de selección que obtendrá el beneficio adicional si está utilizando SQL Server que sea capaz de almacenar en caché el plan de consulta, ya que será la misma cadena acaba paramterised. Si no está utilizando SQL Server, pueden ser necesarios pequeños cambios. También obtienes otro beneficio gratis que es un rendimiento ligeramente mejor a través de menos concatenación de cadenas.

Personalmente agregaría las columnas después del nombre de la tabla Choices, que sería menos propenso a la rotura al agregar nuevas columnas en la tabla de la base de datos (asumiendo los valores predeterminados).

Answer 7

Aunque creo que su código es maravilloso, tengo una lista no exhaustiva de posibles mejoras:

• Estás escribiendo SQL en lugar de utilizar una tecnología de mapeo (NHibernate, LINQ to SQL, EF, subsónico, LLBLGenPro, etc.)
• Concedido que está escribiendo SQL, no está usando parámetros. Al menos está desinfectando correctamente la entrada como lo hace concatenación de cadenas ... con suerte ...
• Concedido que está escribiendo SQL sin parámetros, no está utilizando String.Format para hacer su cadena de caracteres y string- formateo
• Sus nombres de variables son complicados. Usted está nombrando sus variables con una convención húngara de notación húngara. La notación húngara fue originalmente con la intención de ayudar a que sus nombres de variables transmitan cómo estas variables son utilizadas, no para transmitir sus tipos.
• Está codificando la implementación, no la interfaz. Su variable de conexión debe escribirse IDbConnection y su variable de comando debe escribirse IDbCommand. No SqlConnection y SqlCommand. Use IDbConnection.CreateCommand().
• SanitizeInput debe renombrarse como EscapeStringForSQL o algo que indique su intención real. No necesitas escapar de los números.

Answer 8

Haga que el comando SQL sea una cadena separada antes de llamar al constructor. Será más fácil de leer

En lugar de:

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

Haga esto:

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

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

(Perdona mi no saber la sintaxis VB correcta)

Lo que pasa es que el vertido de la cadena literal en el SqlCommand (El constructor simplemente está abarrotando muchas cosas en un gran comando que alguien tiene que romper en su cabeza más adelante.

Además, cuando se desea imprimir el SQL se está ejecutando durante la depuración, será mucho más fácil de lanzar en un simple

Print "Now executing: " + sqlstr 

No se preocupe que va a crear un "extra" variable. La computadora no le importa.