Désolé de ne commenter qu'en premier lieu, mais je poste presque tous les jours un commentaire similaire car beaucoup de gens pensent qu'il serait intelligent d'encapsuler la fonctionnalité ADO.NET dans une DB-Class (moi aussi il y a 10 ans). La plupart du temps, ils décident d'utiliser des objets statiques/partagés car cela semble être plus rapide que de créer un nouvel objet pour n'importe quelle action.
Ce n'est ni une bonne idée en termes de performances ni en termes de sécurité intégrée.
Ne braconnez pas sur le territoire du Connection-Pool
Il y a une bonne raison pour laquelle ADO.NET gère en interne les connexions sous-jacentes au SGBD dans le pool de connexions ADO-NET :
En pratique, la plupart des applications n'utilisent qu'une ou quelques configurations différentes pour les connexions. Cela signifie que pendant l'exécution de l'application, de nombreuses connexions identiques seront ouvertes et fermées à plusieurs reprises. Pour minimiser le coût d'ouverture des connexions, ADO.NET utilise une technique d'optimisation appelée regroupement de connexions.
Le regroupement de connexions réduit le nombre de fois que de nouvelles connexions doivent être ouvertes. Le pooler conserve la propriété de la connexion physique. Il gère les connexions en gardant actif un ensemble de connexions actives pour chaque configuration de connexion donnée. Chaque fois qu'un utilisateur appelle Open sur une connexion, le pooler recherche une connexion disponible dans le pool. Si une connexion groupée est disponible, il la renvoie à l'appelant au lieu d'ouvrir une nouvelle connexion. Lorsque l'application appelle Close sur la connexion, le pooler la renvoie dans le pool de connexions actives au lieu de la fermer. Une fois la connexion renvoyée au pool, elle est prête à être réutilisée lors du prochain appel ouvert.
Donc, évidemment, il n'y a aucune raison d'éviter de créer, d'ouvrir ou de fermer des connexions puisqu'en réalité elles ne sont pas créées, ouvertes et fermées du tout. Il s'agit "seulement" d'un indicateur permettant au pool de connexions de savoir quand une connexion peut être réutilisée ou non. Mais c'est un indicateur très important, car si une connexion est "en cours d'utilisation" (le pool de connexion suppose), une nouvelle connexion physique doit être ouverte au SGBD, ce qui coûte très cher.
Vous n'obtenez donc aucune amélioration des performances, mais le contraire. Si la taille maximale du pool spécifiée (100 est la valeur par défaut) est atteinte, vous obtiendrez même des exceptions (trop de connexions ouvertes ...). Cela aura donc non seulement un impact considérable sur les performances, mais sera également une source d'erreurs désagréables et (sans utiliser Transactions) une zone de vidage de données.
Si vous utilisez même des connexions statiques, vous créez un verrou pour chaque thread essayant d'accéder à cet objet. ASP.NET est un environnement multithreading par nature. Il y a donc de grandes chances que ces verrous causent au mieux des problèmes de performances. En fait, tôt ou tard, vous obtiendrez de nombreuses exceptions différentes (comme votre ExecuteReader nécessite une connexion ouverte et disponible ).
Conclusion :
- Ne réutilisez pas du tout les connexions ou les objets ADO.NET.
- Ne les rendez pas statiques/partagés (dans VB.NET)
- Toujours créer, ouvrir (en cas de connexions), utiliser, fermer et disposer là où vous en avez besoin (par exemple dans une méthode)
- utiliser l'instruction
using-statement
disposer et fermer (en cas de Connexions) implicitement
C'est vrai non seulement pour les connexions (bien que le plus notable). Chaque objet implémentant IDisposable
devrait être éliminé (le plus simple par using-statement
), d'autant plus dans le System.Data.SqlClient
espace de noms.
Tout ce qui précède va à l'encontre d'une classe DB personnalisée qui encapsule et réutilise tous les objets. C'est la raison pour laquelle j'ai commenté de le jeter. Ce n'est qu'une source de problème.
Modifier :Voici une implémentation possible de votre retrievePromotion
-méthode :
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 [email protected]";
using (var da = new SqlDataAdapter(queryString, connection))
{
// you could also use a SqlDataReader instead
// note that a DataTable does not need to be disposed since it does not implement IDisposable
var tblPromotion = new DataTable();
// avoid SQL-Injection
da.SelectCommand.Parameters.Add("@PromotionID", SqlDbType.Int);
da.SelectCommand.Parameters["@PromotionID"].Value = promotionID;
try
{
connection.Open(); // not necessarily needed in this case because DataAdapter.Fill does it otherwise
da.Fill(tblPromotion);
if (tblPromotion.Rows.Count != 0)
{
var promoRow = tblPromotion.Rows[0];
promo = new Promotion()
{
promotionID = promotionID,
promotionTitle = promoRow.Field<String>("PromotionTitle"),
promotionUrl = promoRow.Field<String>("PromotionURL")
};
}
}
catch (Exception ex)
{
// log this exception or throw it up the StackTrace
// we do not need a finally-block to close the connection since it will be closed implicitely in an using-statement
throw;
}
}
}
return promo;
}