¿Está bien pasar SQLCommand como parámetro?

Question

que tienen una capa de negocios que pasa una cadena Conn y una SQLCommand a una capa de datos como tal

public void PopulateLocalData() 
    { 
     System.Data.SqlClient.SqlCommand cmd = new System.Data.SqlClient.SqlCommand(); 
     cmd.CommandType = System.Data.CommandType.StoredProcedure; 
     cmd.CommandText = "usp_PopulateServiceSurveyLocal"; 
     DataLayer.DataProvider.ExecSQL(ConnString, cmd); 
    } 

dataLayer entonces simplemente ejecuta el sql como tal

 public static int ExecSQL(string sqlConnString, System.Data.SqlClient.SqlCommand cmd) 
    { 
     int rowsAffected; 
     using (SqlConnection conn = new SqlConnection(sqlConnString)) 
     { 
      conn.Open(); 
      cmd.Connection = conn; 
      rowsAffected = cmd.ExecuteNonQuery(); 
      cmd.Dispose(); 
     } 
     return rowsAffected; 
    } 

¿Está bien para mí pasar el SQLCommand como un parámetro como este o hay una forma mejor y más aceptada de hacerlo. Una de mis preocupaciones es que si se produce un error al ejecutar la consulta, la línea cmd.dispose nunca se ejecutará. ¿Eso significa que continuará consumiendo memoria que nunca se liberará?

Actualización:

Siguiendo el consejo de Eric dividí más explícitamente el negocio y las capas de datos por lo que el método de la capa de negocio se parece a esto

public void PopulateLocalData() 
    { 
     DataLayer Data = new DataLayer(this.ConnString); 
     Data.UpdateLocalData(); 
    } 

y el método que se llama en el DataLayer tiene este aspecto .

 public void UpdateLocalData() 
    { 
     using (SqlConnection conn = new SqlConnection(this.ConnString)) 
     using(SqlCommand cmd = new SqlCommand()) 
     { 
      cmd.CommandType = System.Data.CommandType.StoredProcedure; 
      cmd.CommandText = "usp_PopulateServiceSurveyLocal"; 
      conn.Open(); 
      cmd.Connection = conn; 
      cmd.ExecuteNonQuery(); 
     } 
    } 

De esta manera, es muy claro que tanto el SQLCommand como el SQLConnection se eliminarán correctamente. Gracias.

Answer 1

Idealmente, su capa empresarial no debe conocer los detalles de implementación de su capa de datos. Por lo tanto, si implementa la capa de datos con los objetos SqlCommand o con algo como NHibernate, debería ser irrelevante para la capa empresarial. Esto hace que sea teóricamente fácil "desplazar" la capa de datos y reemplazarla por otra.

Resumiendo: pasar el SqlCommand de la capa empresarial a la capa de datos es, en mi opinión, no se considera una buena práctica.

En cuanto Dispose(): si está utilizando una declaración usando-(como using(SqlConnection ...)), el método Dispose() se llama automáticamente al final de la instrucción using. No tienes que hacer esto manualmente.

Answer 2

Bueno, para empezar, se puede cambiar a:

public static int ExecSQL(string sqlConnString, System.Data.SqlClient.SqlCommand cmd) 
{ 
    int rowsAffected; 
    try 
    { 
     using (SqlConnection conn = new SqlConnection(sqlConnString)) 
     { 
      conn.Open(); 
      cmd.Connection = conn; 
      rowsAffected = cmd.ExecuteNonQuery(); 
     } 
    } finally { 
     cmd.Dispose(); 
    } 
    return rowsAffected; 
} 

Además, por lo general mis separar las capas de negocio y datos más que tú. Mi capa empresarial llamaría a un método "GetLocalSurvey" en la capa de datos, que manejaría todas las tonterías de SQL.

Answer 3

¿Por qué no lo cambia a esto:

public static int ExecProcedure(string sqlConnString, string procedureName) 
{ 
    using (var cmd = new System.Data.SqlClient.SqlCommand()) 
    { 
     cmd.CommandType = System.Data.CommandType.StoredProcedure; 
     cmd.CommandText = procedureName; 
     int rowsAffected; 
     using (SqlConnection conn = new SqlConnection(sqlConnString)) 
     { 
      conn.Open(); 
      cmd.Connection = conn; 
      return cmd.ExecuteNonQuery(); 
     } 
    } 
} 

¿Quieres parámetros adicionales? Crear sobrecargas, refactorizar. Comparte la mayoría del código en la función común. Crear new System.Data.SqlClient.SqlCommand() en todas partes es un enfoque equivocado.

Answer 4

El que crea el comando debe ser responsable de eliminarlo. La forma más sencilla para esto es para eliminar la llamada a cmd.Dispose de ExecSql y en lugar de llamar a su función como esta:

public void PopulateLocalData() 
{ 
    using (System.Data.SqlClient.SqlCommand cmd = new System.Data.SqlClient.SqlCommand()) 
    { 
     cmd.CommandType = System.Data.CommandType.StoredProcedure; 
     cmd.CommandText = "usp_PopulateServiceSurveyLocal"; 
     DataLayer.DataProvider.ExecSQL(ConnString, cmd); 
    } 
} 

Una de mis preocupaciones es si se produce un error al ejecutar la consulta de la línea se cmd.dispose nunca ejecutar¿Eso significa que continuará consumiendo memoria que nunca se liberará?

Casualmente, SqlClient.SqlCommand no es necesario desecharlo. Esto, sin embargo, es un detalle de implementación que no se debe confiar - la regla general sigue siendo: Si implementa IDisposable, desecharlo (SqlCeClient.SqlCeCommand, por ejemplo, hace necesidad de ser eliminados ...)