Lo siento por comentar solo en primer lugar, pero estoy publicando casi todos los días un comentario similar desde que muchas personas piensan que sería inteligente encapsular la funcionalidad de ADO.NET en una clase de base de datos (yo también hace 10 años). En su mayoría deciden usar objetos estáticos/compartidos ya que parece ser más rápido que crear un nuevo objeto para cualquier acción.
Eso no es ni una buena idea en términos de rendimiento ni en términos de seguridad contra fallos.
No te entrometas en el territorio del Connection-Pool
Hay una buena razón por la cual ADO.NET gestiona internamente las conexiones subyacentes al DBMS en el Connection-Pool de ADO-NET:
En la práctica, la mayoría de las aplicaciones utilizan solo una o unas pocas configuraciones diferentes para las conexiones. Esto significa que durante la ejecución de la aplicación, muchas conexiones idénticas se abrirán y cerrarán repetidamente. Para minimizar el costo de abrir conexiones, ADO.NET utiliza una técnica de optimización llamada connection pooling.
El connection pooling reduce la cantidad de veces que se deben abrir nuevas conexiones. El pooler mantiene la propiedad de la conexión física. Administra las conexiones manteniendo un conjunto de conexiones activas para cada configuración de conexión dada. Cada vez que un usuario llama a Open en una conexión, el pooler busca una conexión disponible en el grupo. Si hay una conexión en el pool, la devuelve al llamante en lugar de abrir una nueva conexión. Cuando la aplicación llama a Close en la conexión, el pooler la devuelve al conjunto de conexiones activas en lugar de cerrarla. Una vez que la conexión se devuelve al pool, está lista para ser reutilizada en la siguiente llamada a Open.
Por lo tanto, obviamente no hay razón para evitar crear, abrir o cerrar conexiones ya que en realidad no se crean, abren ni cierran en absoluto. Esto es "solo" una bandera para que el pool de conexiones sepa cuándo se puede reutilizar una conexión o no. Pero es una bandera muy importante, porque si una conexión está "en uso" (lo asume el pool de conexiones), se debe abrir una nueva conexión física al DBMS, lo que es muy costoso.
Por lo tanto, no estás obteniendo ninguna mejora de rendimiento, sino lo contrario. Si se alcanza el tamaño máximo de pool especificado (100 es el valor predeterminado), incluso obtendrías excepciones (demasiadas conexiones abiertas ...). Por lo tanto, esto no solo impactará enormemente en el rendimiento sino que también será una fuente de errores desagradables y (sin usar Transacciones) un área de volcado de datos.
Incluso si estás usando conexiones estáticas, estás creando un bloqueo para cada hilo que intente acceder a este objeto. ASP.NET es un entorno multitarea por naturaleza. Así que hay una gran posibilidad de que estos bloqueos causen problemas de rendimiento en el mejor de los casos. En realidad, tarde o temprano obtendrás muchas excepciones diferentes (como tu ExecuteReader requiere una conexión abierta y disponible).
Conclusión:
- No reutilices conexiones ni ningún objeto de ADO.NET en absoluto.
- No los hagas estáticos/compartidos (en VB.NET)
- Siempre crea, abre (en caso de Conexiones), usa, cierra y desecha donde los necesites (por ejemplo, en un método)
- usa la
using-statement para desechar y cerrar (en caso de Conexiones) de forma implícita
Esto es cierto no solo para las Conexiones (aunque es más notorio). Cada objeto que implemente IDisposable debería ser desechado (más simple con un using-statement), mucho más en el espacio de nombres System.Data.SqlClient.
Todo lo anterior habla en contra de una clase de base de datos personalizada que encapsula y reutiliza todos los objetos. Esa es la razón por la que comenté para desecharlo. Eso solo es una fuente de problemas.
Editar: Aquí hay una posible implementación de tu método retrievePromotion:
public Promotion retrievePromotion(int promotionID)
{
Promotion promo = null;
var connectionString = System.Configuration.ConfigurationManager.ConnectionStrings["MainConnStr"].ConnectionString;
using (SqlConnection connection = new SqlConnection(connectionString))
{
var queryString = "SELECT PromotionID, PromotionTitle, PromotionURL FROM Promotion WHERE PromotionID=@PromotionID";
using (var da = new SqlDataAdapter(queryString, connection))
{
// también podrías usar un SqlDataReader en su lugar
// ten en cuenta que un DataTable no necesita ser desechado ya que no implementa IDisposable
var tblPromotion = new DataTable();
// evitar la inyección SQL
da.SelectCommand.Parameters.Add("@PromotionID", SqlDbType.Int);
da.SelectCommand.Parameters["@PromotionID"].Value = promotionID;
try
{
connection.Open(); // no necesariamente necesario en este caso porque DataAdapter.Fill lo hace de otra manera
da.Fill(tblPromotion);
if (tblPromotion.Rows.Count != 0)
{
var promoRow = tblPromotion.Rows[0];
promo = new Promotion()
{
promotionID = promotionID,
promotionTitle = promoRow.Field("PromotionTitle"),
promotionUrl = promoRow.Field("PromotionURL")
};
}
}
catch (Exception ex)
{
// registra esta excepción o lánzala en la StackTrace
// no necesitamos un bloque finally para cerrar la conexión ya que se cerrará implícitamente en un using-statement
throw;
}
}
}
return promo;
}
